KAFKA-17423 Trie implementation #17087
Conversation
…asses. Fixed tests
| * Creates an empty tree. | ||
| */ | ||
| public Trie() { | ||
| clear(); |
There was a problem hiding this comment.
There's compilation error please check.
https://github.com/apache/kafka/actions/runs/10722721527/job/29734313780?pr=17087#step:6:2650
| * @throws IndexOutOfBoundsException if bitIndex specifies a bit not in the range being tracked. | ||
| */ | ||
| public static boolean contains(final int[] bitMaps, final int bitIndex) { | ||
| return (bitMaps[getIntIndex(bitIndex)] & getIntBit(bitIndex)) != 0; |
There was a problem hiding this comment.
I think we can remove the != 0 but still remains the logic.
There was a problem hiding this comment.
Should check whether the index is out of range or not.
There was a problem hiding this comment.
The Bitmap checks are performed on every ACL in the Trie during some operations. While it may make sense to add range checks on a general case library, this is that that. This is a specific case where we know the values of the indexes before we start and where we want to reduce the overhead as much as possible.
| * @param numberOfBits the number of bits to store in the array of bit maps. | ||
| * @return the number of bit maps necessary. | ||
| */ | ||
| public static int numberOfBitMaps(final int numberOfBits) { |
There was a problem hiding this comment.
Should we add some conditional check to avoid numberOfBits is a negative value?
There was a problem hiding this comment.
The Bitmap checks are performed on every ACL in the Trie during some operations. While it may make sense to add range checks on a general case library, this is that that. This is a specific case where we know the values of the indexes before we start and where we want to reduce the overhead as much as possible.
| * @throws IndexOutOfBoundsException if bitIndex specifies a bit not in the range being tracked. | ||
| */ | ||
| public static void set(final int[] bitMaps, final int bitIndex) { | ||
| bitMaps[getIntIndex(bitIndex)] |= getIntBit(bitIndex); |
There was a problem hiding this comment.
Should check whether the index is out of range or not.
There was a problem hiding this comment.
The Bitmap checks are performed on every ACL in the Trie during some operations. While it may make sense to add range checks on a general case library, this is that that. This is a specific case where we know the values of the indexes before we start and where we want to reduce the overhead as much as possible.
| * @throws IndexOutOfBoundsException if bitIndex specifies a bit not in the range being tracked. | ||
| */ | ||
| public static boolean contains(final int bitMap, final int bitIndex) { | ||
| return (bitMap & getIntBit(bitIndex)) != 0; |
There was a problem hiding this comment.
Should check whether the index is out of range or not.
There was a problem hiding this comment.
The Bitmap checks are performed on every ACL in the Trie during some operations. While it may make sense to add range checks on a general case library, this is that that. This is a specific case where we know the values of the indexes before we start and where we want to reduce the overhead as much as possible.
| @@ -0,0 +1,70 @@ | |||
| /* | |||
There was a problem hiding this comment.
Maybe change the filename to PackageInfo.java and also what's the purpose of this file?
There was a problem hiding this comment.
This file is used by the javadoc process to create package information. The name is hard coded into the javadoc tool.
This change is dependent upon KAFKA-17316: Standard authorizer refactor (#16779) That change will reduce the number of files that are changed in this request.
More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.
Implements the StandardAuthorizer with a Trie data structure instead of the original sorted list. This results in an order of magnitude speed increase.
Extensive tests are added to verify that the new implementation matches the old implementation.
Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.
Committer Checklist (excluded from commit message)