Copilot commented on code in PR #11683:
URL: https://github.com/apache/ozone/pull/11683#discussion_r4404849023
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java:
##########
@@ -313,6 +313,13 @@ public ByteString getAclByteString() {
return UnsafeByteOperations.unsafeWrap(bytes);
}
+ /**
+ * @return true if the rights include bits that are not a known {@link
ACLType}
+ */
+ public boolean hasUnknownRights() {
+ return (aclBits >>> ACLType.values().length) != 0;
Review Comment:
`ACLType.values().length` allocates a new array every time
`hasUnknownRights()` is called. Since this method is now invoked on request
paths (`preExecute`), consider caching the ACL-type count in a `static final
int` (or similar) to avoid repeated allocations. Also, using a precomputed mask
for ‘known bits’ makes the intent clearer and avoids edge cases if the enum
size grows (e.g., shift-by-32 semantics).
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java:
##########
@@ -146,7 +146,8 @@ public static ACLType getACLRight(String type) {
*/
public static String getACLString(BitSet acls) {
StringBuffer sb = new StringBuffer();
- acls.stream().forEach(acl -> {
+ // Skip bits that are not a known ACL type.
+ acls.stream().filter(acl -> acl < ACLType.values().length).forEach(acl
-> {
sb.append(getAclString(ACLType.values()[acl]));
Review Comment:
`ACLType.values()` allocates a new array on each call. In this stream it’s
called once per element in the filter and again in the body, which can add
noticeable overhead when ACLs are frequently stringified (e.g., audit logs).
Cache the `ACLType[] values = ACLType.values()` and `int len = values.length`
before streaming, then reference `len` and `values[acl]` inside the lambda.
##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclStorage.java:
##########
@@ -58,10 +56,10 @@ public static OzoneAclInfo toProtobuf(OzoneAcl acl) {
public static OzoneAcl fromProtobuf(OzoneAclInfo protoAcl) {
BitSet aclRights = BitSet.valueOf(protoAcl.getRights().toByteArray());
- List<IAccessAuthorizer.ACLType> aclTypeList = aclRights.stream()
- .mapToObj(a -> IAccessAuthorizer.ACLType.values()[a])
- .collect(Collectors.toList());
- EnumSet<IAccessAuthorizer.ACLType> aclSet = EnumSet.copyOf(aclTypeList);
+ EnumSet<IAccessAuthorizer.ACLType> aclSet =
EnumSet.noneOf(IAccessAuthorizer.ACLType.class);
+ // Skip bits that are not a known ACL type.
+ aclRights.stream().filter(a -> a <
IAccessAuthorizer.ACLType.values().length)
+ .forEach(a -> aclSet.add(IAccessAuthorizer.ACLType.values()[a]));
Review Comment:
Same `enum.values()` allocation issue as above:
`IAccessAuthorizer.ACLType.values()` gets called repeatedly inside the stream
pipeline. Cache `IAccessAuthorizer.ACLType[] values =
IAccessAuthorizer.ACLType.values()` (and `int len = values.length`) once, then
use `len` in the filter and `values[a]` in `forEach`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
