Skip to content

Conversation

@dlmarion
Copy link
Contributor

I removed the methods from ByteUtils and reverted the places where the methods were called back to their original logic, but retained the use of the ByteUtils static variables. This yielded no significant performance difference. I re-ran the benchmarks on main before #27 was merged, the HEAD of main, and this branch. I ran mvn clean verify on each branch and ran the benchmark using the command:

taskset -c 1 mvn exec:exec -Dexec.executable="java" -Dexec.classpathScope=test -Dexec.args="-classpath %classpath org.apache.accumulo.access.AccessExpressionBenchmark"

Here are the results:

main @c959026c8631da8691764b3f20a9148b84bafc99:
AccessExpressionBenchmark.measureBytesParsing          thrpt   12  12.665 ± 0.225  ops/us
AccessExpressionBenchmark.measureEvaluation            thrpt   12  16.663 ± 0.441  ops/us
AccessExpressionBenchmark.measureEvaluationAndParsing  thrpt   12   8.435 ± 0.117  ops/us
AccessExpressionBenchmark.measureStringParsing         thrpt   12  11.709 ± 0.225  ops/us

main:
AccessExpressionBenchmark.measureBytesParsing          thrpt   12  12.864 ± 0.278  ops/us
AccessExpressionBenchmark.measureEvaluation            thrpt   12  16.849 ± 0.312  ops/us
AccessExpressionBenchmark.measureEvaluationAndParsing  thrpt   12   8.480 ± 0.094  ops/us
AccessExpressionBenchmark.measureStringParsing         thrpt   12  11.482 ± 0.033  ops/us


test-perf-fixes:
AccessExpressionBenchmark.measureBytesParsing          thrpt   12  12.555 ± 0.196  ops/us
AccessExpressionBenchmark.measureEvaluation            thrpt   12  16.651 ± 0.730  ops/us
AccessExpressionBenchmark.measureEvaluationAndParsing  thrpt   12   8.432 ± 0.094  ops/us
AccessExpressionBenchmark.measureStringParsing         thrpt   12  11.601 ± 0.209  ops/us

@dlmarion dlmarion requested a review from DomGarguilo October 26, 2023 20:22
@dlmarion dlmarion self-assigned this Oct 26, 2023
@dlmarion
Copy link
Contributor Author

@DomGarguilo - I'm going to close this I think as there is no value-add in these changes. I did find a place (AccessEvaluatorImpl::71) where a literal is being used instead of the static variable.

@dlmarion dlmarion closed this Oct 26, 2023
@ctubbsii ctubbsii added this to the 1.0.0 milestone Oct 27, 2023
@ctubbsii ctubbsii removed this from the 1.0.0 milestone Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants