lucasbru commented on code in PR #18293: URL: https://github.com/apache/kafka/pull/18293#discussion_r2197166329
########## jmh-benchmarks/src/main/java/org/apache/kafka/jmh/acl/AuthorizerBenchmark.java: ########## @@ -89,13 +90,19 @@ public class AuthorizerBenchmark { private List<Action> actions = new ArrayList<>(); private RequestContext authorizeContext; private RequestContext authorizeByResourceTypeContext; + private AclBindingFilter filter; + private AclOperation op; + private ResourceType resourceType; Random rand = new Random(System.currentTimeMillis()); double eps = 1e-9; @Setup(Level.Trial) public void setup() throws Exception { authorizer = new StandardAuthorizer(); + filter = AclBindingFilter.ANY; + op = AclOperation.READ; + resourceType = ResourceType.TOPIC; Review Comment: Why make them global? ########## jmh-benchmarks/src/main/java/org/apache/kafka/jmh/acl/AuthorizerBenchmark.java: ########## @@ -204,17 +211,17 @@ public void tearDown() throws IOException { } @Benchmark - public void testAclsIterator() { - authorizer.acls(AclBindingFilter.ANY); + public Iterable<AclBinding> testAclsIterator() { + return authorizer.acls(filter); } @Benchmark - public void testAuthorizer() { - authorizer.authorize(authorizeContext, actions); + public List<AuthorizationResult> testAuthorizer() { + return authorizer.authorize(authorizeContext, actions); } @Benchmark - public void testAuthorizeByResourceType() { - authorizer.authorizeByResourceType(authorizeByResourceTypeContext, AclOperation.READ, ResourceType.TOPIC); + public AuthorizationResult testAuthorizeByResourceType() { + return authorizer.authorizeByResourceType(authorizeByResourceTypeContext, op, resourceType); Review Comment: alternatively, we could use a blackhole. Won't this show up as a warning "unused parameter" in INtelliJ? ########## jmh-benchmarks/src/main/java/org/apache/kafka/jmh/acl/StandardAuthorizerUpdateBenchmark.java: ########## @@ -55,8 +55,8 @@ @State(Scope.Benchmark) @Fork(value = 1) -@Warmup(iterations = 0) -@Measurement(iterations = 4) +@Warmup(iterations = 7, timeUnit = TimeUnit.SECONDS, time = 2) Review Comment: why are we changin this? -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org