Re: RFR: 8284880: Re-examine sun.invoke.util.Wrapper hash tables [v2]
On Thu, 14 Apr 2022 13:59:57 GMT, Claes Redestad wrote: >> This patch examines and optimizes `Wrapper` lookups. >> >> First wrote a few simple microbenchmarks to verify there are actual speedups >> from using perfect hash tables in `sun.invoke.util.Wrapper` compared to >> simpler lookup mechanisms (such as if-else or switch). Turns out there _is_ >> a speed-up for the case of `char` -> `Wrapper`, but not when mapping from >> `Class` -> `Wrapper`, so let's drop those. The `forPrimitiveType` case >> didn't use the `FROM_CHAR` table for some reason, which is remedied. >> >> Micros show benefits across the board for warmed up case: >> >> >> Baseline, OOTB >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 14.387 ? 0.127 ns/op >> Wrappers.forPrimitive avgt5 38.818 ? 0.592 ns/op >> Wrappers.forPrimitiveType avgt5 26.085 ? 2.291 ns/op >> Wrappers.forWrapperavgt5 44.459 ? 1.635 ns/op >> >> Patch, OOTB >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 14.357 ? 0.133 ns/op >> Wrappers.forPrimitive avgt5 23.930 ? 0.071 ns/op >> Wrappers.forPrimitiveType avgt5 14.343 ? 0.017 ns/op >> Wrappers.forWrapperavgt5 27.622 ? 0.022 ns/op >> >> >> For `-Xint` case (`Wrapper` use is prominent during warmup, e.g., when >> spinning up of MHs) there are decent or even great wins in all cases but >> `forPrimitiveType` - which was changed from a simple switch to use the hash >> lookup. Since the interpreter penalty is small in absolute terms and the win >> on JITed code is significant this seems like a reasonable trade-off: >> >> >> Baseline, -Xint >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 1246.144 ? 149.933 ns/op >> Wrappers.forPrimitive avgt5 4955.297 ? 329.869 ns/op >> Wrappers.forPrimitiveType avgt5 716.840 ? 62.568 ns/op >> Wrappers.forWrapperavgt5 5774.700 ? 367.627 ns/op >> >> Patch, -Xint >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 1068.096 ? 101.728 ns/op >> Wrappers.forPrimitive avgt5 1146.670 ? 59.142 ns/op >> Wrappers.forPrimitiveType avgt5 998.037 ? 118.144 ns/op >> Wrappers.forWrapperavgt5 3581.226 ? 20.167 ns/op > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add line break in make/test/BuildMicrobenchmark.gmk > > Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> Thanks for reviews! - PR: https://git.openjdk.java.net/jdk/pull/8242
Re: RFR: 8284880: Re-examine sun.invoke.util.Wrapper hash tables [v2]
On Thu, 14 Apr 2022 13:59:57 GMT, Claes Redestad wrote: >> This patch examines and optimizes `Wrapper` lookups. >> >> First wrote a few simple microbenchmarks to verify there are actual speedups >> from using perfect hash tables in `sun.invoke.util.Wrapper` compared to >> simpler lookup mechanisms (such as if-else or switch). Turns out there _is_ >> a speed-up for the case of `char` -> `Wrapper`, but not when mapping from >> `Class` -> `Wrapper`, so let's drop those. The `forPrimitiveType` case >> didn't use the `FROM_CHAR` table for some reason, which is remedied. >> >> Micros show benefits across the board for warmed up case: >> >> >> Baseline, OOTB >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 14.387 ? 0.127 ns/op >> Wrappers.forPrimitive avgt5 38.818 ? 0.592 ns/op >> Wrappers.forPrimitiveType avgt5 26.085 ? 2.291 ns/op >> Wrappers.forWrapperavgt5 44.459 ? 1.635 ns/op >> >> Patch, OOTB >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 14.357 ? 0.133 ns/op >> Wrappers.forPrimitive avgt5 23.930 ? 0.071 ns/op >> Wrappers.forPrimitiveType avgt5 14.343 ? 0.017 ns/op >> Wrappers.forWrapperavgt5 27.622 ? 0.022 ns/op >> >> >> For `-Xint` case (`Wrapper` use is prominent during warmup, e.g., when >> spinning up of MHs) there are decent or even great wins in all cases but >> `forPrimitiveType` - which was changed from a simple switch to use the hash >> lookup. Since the interpreter penalty is small in absolute terms and the win >> on JITed code is significant this seems like a reasonable trade-off: >> >> >> Baseline, -Xint >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 1246.144 ? 149.933 ns/op >> Wrappers.forPrimitive avgt5 4955.297 ? 329.869 ns/op >> Wrappers.forPrimitiveType avgt5 716.840 ? 62.568 ns/op >> Wrappers.forWrapperavgt5 5774.700 ? 367.627 ns/op >> >> Patch, -Xint >> Benchmark Mode Cnt Score Error Units >> Wrappers.forBasicType avgt5 1068.096 ? 101.728 ns/op >> Wrappers.forPrimitive avgt5 1146.670 ? 59.142 ns/op >> Wrappers.forPrimitiveType avgt5 998.037 ? 118.144 ns/op >> Wrappers.forWrapperavgt5 3581.226 ? 20.167 ns/op > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add line break in make/test/BuildMicrobenchmark.gmk > > Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> Looks good.The copyright header end-year needs update before you push. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8242
Re: RFR: 8284880: Re-examine sun.invoke.util.Wrapper hash tables [v2]
> This patch examines and optimizes `Wrapper` lookups. > > First wrote a few simple microbenchmarks to verify there are actual speedups > from using perfect hash tables in `sun.invoke.util.Wrapper` compared to > simpler lookup mechanisms (such as if-else or switch). Turns out there _is_ a > speed-up for the case of `char` -> `Wrapper`, but not when mapping from > `Class` -> `Wrapper`, so let's drop those. The `forPrimitiveType` case > didn't use the `FROM_CHAR` table for some reason, which is remedied. > > Micros show benefits across the board for warmed up case: > > > Baseline, OOTB > Benchmark Mode Cnt Score Error Units > Wrappers.forBasicType avgt5 14.387 ? 0.127 ns/op > Wrappers.forPrimitive avgt5 38.818 ? 0.592 ns/op > Wrappers.forPrimitiveType avgt5 26.085 ? 2.291 ns/op > Wrappers.forWrapperavgt5 44.459 ? 1.635 ns/op > > Patch, OOTB > Benchmark Mode Cnt Score Error Units > Wrappers.forBasicType avgt5 14.357 ? 0.133 ns/op > Wrappers.forPrimitive avgt5 23.930 ? 0.071 ns/op > Wrappers.forPrimitiveType avgt5 14.343 ? 0.017 ns/op > Wrappers.forWrapperavgt5 27.622 ? 0.022 ns/op > > > For `-Xint` case (`Wrapper` use is prominent during warmup, e.g., when > spinning up of MHs) there are decent or even great wins in all cases but > `forPrimitiveType` - which was changed from a simple switch to use the hash > lookup. Since the interpreter penalty is small in absolute terms and the win > on JITed code is significant this seems like a reasonable trade-off: > > > Baseline, -Xint > Benchmark Mode Cnt Score Error Units > Wrappers.forBasicType avgt5 1246.144 ? 149.933 ns/op > Wrappers.forPrimitive avgt5 4955.297 ? 329.869 ns/op > Wrappers.forPrimitiveType avgt5 716.840 ? 62.568 ns/op > Wrappers.forWrapperavgt5 5774.700 ? 367.627 ns/op > > Patch, -Xint > Benchmark Mode Cnt Score Error Units > Wrappers.forBasicType avgt5 1068.096 ? 101.728 ns/op > Wrappers.forPrimitive avgt5 1146.670 ? 59.142 ns/op > Wrappers.forPrimitiveType avgt5 998.037 ? 118.144 ns/op > Wrappers.forWrapperavgt5 3581.226 ? 20.167 ns/op Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Add line break in make/test/BuildMicrobenchmark.gmk Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Changes: - all: https://git.openjdk.java.net/jdk/pull/8242/files - new: https://git.openjdk.java.net/jdk/pull/8242/files/d21330bb..97eec9d3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8242=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8242=00-01 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8242.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8242/head:pull/8242 PR: https://git.openjdk.java.net/jdk/pull/8242
