[
https://issues.apache.org/jira/browse/KAFKA-9685?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17070153#comment-17070153
]
ASF GitHub Bot commented on KAFKA-9685:
---------------------------------------
lbradstreet commented on pull request #8382: KAFKA-9685: PT2, avoid unnecessary
set creation in ACL matching
URL: https://github.com/apache/kafka/pull/8382
https://github.com/apache/kafka/pull/8261 went a long way to solving some of
the ACL performance issues. I don't think we need to create sets at all for the
`find` and `isEmpty` calls.
```
Before:
Benchmark (aclCount) (resourceCount) Mode
Cnt Score Error Units
AclAuthorizerBenchmark.testAclsIterator 5 5000 avgt
15 0.430 ± 0.004 ms/op
AclAuthorizerBenchmark.testAclsIterator 5 10000 avgt
15 0.980 ± 0.007 ms/op
AclAuthorizerBenchmark.testAclsIterator 5 50000 avgt
15 11.191 ± 0.032 ms/op
AclAuthorizerBenchmark.testAclsIterator 10 5000 avgt
15 0.880 ± 0.007 ms/op
AclAuthorizerBenchmark.testAclsIterator 10 10000 avgt
15 2.642 ± 0.029 ms/op
AclAuthorizerBenchmark.testAclsIterator 10 50000 avgt
15 26.361 ± 0.242 ms/op
AclAuthorizerBenchmark.testAclsIterator 15 5000 avgt
15 1.655 ± 0.024 ms/op
AclAuthorizerBenchmark.testAclsIterator 15 10000 avgt
15 5.276 ± 0.041 ms/op
AclAuthorizerBenchmark.testAclsIterator 15 50000 avgt
15 40.702 ± 0.574 ms/op
AclAuthorizerBenchmark.testAuthorizer 5 5000 avgt
15 0.202 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 5 10000 avgt
15 0.233 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 5 50000 avgt
15 0.424 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 10 5000 avgt
15 0.202 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 10 10000 avgt
15 0.253 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 10 50000 avgt
15 0.423 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 15 5000 avgt
15 0.198 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 15 10000 avgt
15 0.242 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 15 50000 avgt
15 0.391 ± 0.002 ms/op
JMH benchmarks done
After:
Benchmark (aclCount) (resourceCount) Mode
Cnt Score Error Units
AclAuthorizerBenchmark.testAclsIterator 5 5000 avgt
15 0.504 ± 0.164 ms/op
AclAuthorizerBenchmark.testAclsIterator 5 10000 avgt
15 1.038 ± 0.271 ms/op
AclAuthorizerBenchmark.testAclsIterator 5 50000 avgt
15 11.767 ± 0.028 ms/op
AclAuthorizerBenchmark.testAclsIterator 10 5000 avgt
15 0.827 ± 0.016 ms/op
AclAuthorizerBenchmark.testAclsIterator 10 10000 avgt
15 2.801 ± 0.027 ms/op
AclAuthorizerBenchmark.testAclsIterator 10 50000 avgt
15 26.157 ± 0.191 ms/op
AclAuthorizerBenchmark.testAclsIterator 15 5000 avgt
15 1.814 ± 0.053 ms/op
AclAuthorizerBenchmark.testAclsIterator 15 10000 avgt
15 5.420 ± 0.065 ms/op
AclAuthorizerBenchmark.testAclsIterator 15 50000 avgt
15 41.372 ± 0.659 ms/op
AclAuthorizerBenchmark.testAuthorizer 5 5000 avgt
15 0.064 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 5 10000 avgt
15 0.070 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 5 50000 avgt
15 0.240 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 10 5000 avgt
15 0.055 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 10 10000 avgt
15 0.084 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 10 50000 avgt
15 0.249 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 15 5000 avgt
15 0.057 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 15 10000 avgt
15 0.084 ± 0.001 ms/op
AclAuthorizerBenchmark.testAuthorizer 15 50000 avgt
15 0.243 ± 0.001 ms/op
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Solve Set concatenation perf issue in AclAuthorizer
> ---------------------------------------------------
>
> Key: KAFKA-9685
> URL: https://issues.apache.org/jira/browse/KAFKA-9685
> Project: Kafka
> Issue Type: Improvement
> Components: security
> Affects Versions: 1.1.0
> Reporter: Jiao Zhang
> Assignee: Jiao Zhang
> Priority: Minor
> Fix For: 2.6.0
>
>
> In version 1.1,
> [https://github.com/apache/kafka/blob/71b1e19fc60b5e1f9bba33025737ec2b7fb1c2aa/core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala#L110]
> the logic for checking acls is preparing a merged acl Set with
> {code:java}
> acls = getAcls(new Resource(resource.resourceType,
> Resource.WildCardResource)) ++ getAcls(resource);{code}
> and then pass it as aclMatch's parameter.
> We found scala's Set ++ operation is very slow for example in the case that
> the Set on right hand of ++ has more than 100 entries.
> And the bad performance of ++ is due to iterating every entry of the Set on
> right hand of ++, in which the calculation of HashCode seems heavy.
> The performance of 'authorize' is important as each request delivered to
> broker goes through the logic, that's the reason we can't leave it as-is
> although the change for this proposal seems trivial.
> Here is the approach. We propose to solve this issue by introducing a new
> class 'AclSets' which takes multiple Sets as parameters and do 'find' against
> them one by one.
> {code:java}
> class AclSets(sets: Set[Acl]*){
> def find(p: Acl => Boolean): Option[Acl] =
> sets.flatMap(_.find(p)).headOption
> def isEmpty: Boolean = !sets.exists(_.nonEmpty)
> }
> {code}
> This approach avoid the Set ++ operation like following,
> {code:java}
> val acls = new AclSets(getAcls(new Resource(resource.resourceType,
> Resource.WildCardResource)), getAcls(resource)){code}
> and thus outperforms a lot compared to old logic.
> The benchmark result(we did the test with kafka version 1.1) shows notable
> difference under the condition:
> 1. set on left consists of 60 entries
> 2. set of right consists of 30 entries
> 3. search for absent entry (so that all entries are iterated)
> Benchmark Results is as following.
> Mode Cnt Score
> Error Units
> ScalaSetConcatination.Set thrpt 3 281.974 ± 140.029 ops/ms
> ScalaSetConcatination.AclSets thrpt 3 887.426 ± 40.261 ops/ms
> As the upstream also use the similar ++ operation,
> [https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/security/authorizer/AclAuthorizer.scala#L360]
> we think it's necessary to fix this issue.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
