Skip to content

Commit

Permalink
Optimize inquire.IsSubset (#714)
Browse files Browse the repository at this point in the history
This change set makes IsSubset not use reflection
for faster processing.

FAB-17519 raised a concern that the peer's CPU spikes
during evaluation of lots of combinations, and the captured
CPU profile trace showed that reflection eats a lot of CPU cycles.

Change-Id: I5859d585ae76f1cab204daac7ea461a60fe8f33d
Signed-off-by: yacovm <yacovm@il.ibm.com>
  • Loading branch information
yacovm authored Feb 22, 2020
1 parent bb46909 commit 371c435
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 3 deletions.
55 changes: 55 additions & 0 deletions common/policies/inquire/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ type ComparablePrincipal struct {
mspID string
}

// Equal returns whether this ComparablePrincipal is equal to the given ComparablePrincipal.
func (cp *ComparablePrincipal) Equal(cp2 *ComparablePrincipal) bool {
return cp.mspID == cp2.mspID &&
cp.equalPrincipals(cp2) && cp.equalRoles(cp2) && cp.equalOUs(cp2)
}

// NewComparablePrincipal creates a ComparablePrincipal out of the given MSPPrincipal.
// Returns nil if a failure occurs.
func NewComparablePrincipal(principal *msp.MSPPrincipal) *ComparablePrincipal {
Expand Down Expand Up @@ -180,3 +186,52 @@ func (cps ComparablePrincipalSet) Clone() ComparablePrincipalSet {
}
return res
}

func (cp *ComparablePrincipal) equalRoles(cp2 *ComparablePrincipal) bool {
if cp.role == nil && cp2.role == nil {
return true
}

if cp.role == nil || cp2.role == nil {
return false
}

return cp.role.MspIdentifier == cp2.role.MspIdentifier &&
cp.role.Role == cp2.role.Role
}

func (cp *ComparablePrincipal) equalOUs(cp2 *ComparablePrincipal) bool {
if cp.ou == nil && cp2.ou == nil {
return true
}

if cp.ou == nil || cp2.ou == nil {
return false
}

if cp.ou.OrganizationalUnitIdentifier != cp2.ou.OrganizationalUnitIdentifier {
return false
}

if cp.ou.MspIdentifier != cp2.ou.MspIdentifier {
return false
}

return bytes.Equal(cp.ou.CertifiersIdentifier, cp2.ou.CertifiersIdentifier)
}

func (cp *ComparablePrincipal) equalPrincipals(cp2 *ComparablePrincipal) bool {
if cp.principal == nil && cp2.principal == nil {
return true
}

if cp.principal == nil || cp2.principal == nil {
return false
}

if cp.principal.PrincipalClassification != cp2.principal.PrincipalClassification {
return false
}

return bytes.Equal(cp.principal.Principal, cp2.principal.Principal)
}
4 changes: 1 addition & 3 deletions common/policies/inquire/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ SPDX-License-Identifier: Apache-2.0
package inquire

import (
"reflect"

"github.com/hyperledger/fabric/common/policies"
)

Expand Down Expand Up @@ -275,7 +273,7 @@ func (cps ComparablePrincipalSet) IsSubset(sets ComparablePrincipalSet) bool {
for _, p1 := range cps {
var found bool
for _, p2 := range sets {
if reflect.DeepEqual(p1, p2) {
if p1.Equal(p2) {
found = true
}
}
Expand Down
8 changes: 8 additions & 0 deletions common/policies/inquire/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ func TestMergeSubsetPrincipalSets(t *testing.T) {
assert.True(t, expected.IsEqual(merged))
}

func TestEqual(t *testing.T) {
member1 := NewComparablePrincipal(member("Org1MSP"))
member2 := NewComparablePrincipal(member("Org2MSP"))
anotherMember1 := NewComparablePrincipal(member("Org1MSP"))
assert.False(t, member1.Equal(member2))
assert.True(t, member1.Equal(anotherMember1))
}

func TestIsSubset(t *testing.T) {
members12 := ComparablePrincipalSet{member1, member2, member2}
members321 := ComparablePrincipalSet{member3, member2, member1}
Expand Down

0 comments on commit 371c435

Please sign in to comment.