KAFKA-16669: Remove extra collection copy when generating DescribeAclsResource #15924
Conversation
|
@chiacyu we don't need to collect all items to a new collection, right? |
| ResourcePattern key = entry.getKey(); | ||
| List<AclDescription> aclDescriptions = new ArrayList<>(); | ||
| List<AclDescription> aclDescriptions = new ArrayList<>(128); |
There was a problem hiding this comment.
why not using entry.getValue().size()?
| @@ -69,7 +69,7 @@ class AclApis(authHelper: AuthHelper, | |||
| case Some(auth) => | |||
| val filter = describeAclsRequest.filter | |||
| val returnedAcls = new util.HashSet[AclBinding]() | |||
| auth.acls(filter).forEach(returnedAcls.add) | |||
| returnedAcls.add(auth.acls(filter).iterator().next()) | |||
There was a problem hiding this comment.
Originally we will add all auth.acls(filter) content into returnedAcls, but after this change, we only add the first element. Why do we need this change?
There was a problem hiding this comment.
OK, I know why you did this change now.
we don't need to collect all items to a new collection, right?
I believe what @chia7712 meant is we don't have to do a collection copy here because we already allow passing Iterable into aclsResource method. To make it clear, in the JIRA, we said:
- Iterable -> HashSet
This is the collection copy we want to avoid here. So we don't need returnedAcls anymore, we can pass the result of auth.acls(filter) into aclsResource directly. Is that clear?
This pr reduced the occurrence of collection copy when generating DescribeAclsResource.