I tried to create a JMH to show the difference between various injection techniques. While I am familiar with JMH and how to write them, I make no claims that this is perfect. Also, I did not do any kind of analysis, such as looking into the hot methods assembly.
I made eight different benchmarks, covering the following: 1. Set field directly (control). 2. Set field via setter method (alternate control). 3. Set field via reflection using Field object, not synchronized. 4. Set field via reflection using Method object for setter method, not synchronized. 5. Set field via reflection using Field object, synchronized. 6. Set field via reflection using Method object for setter method, synchronized. 7. Set field via MethodHandle using a handle to the field setter. 8. Set field via MethodHandle using a handle to the setter method. You can see the full results & code at [1]. I only kept a few of the best (lowest error value) runs. One of the best runs I have copied below, but they are best viewed in monospace font, like at [1]. These numbers are specific to my machine, so the important inference that can be gained from these data is the relative, not absolute values. Benchmark (str) Mode Cnt Score Error Units SynchronizedBenchmark.viaField str avgt 15 7.158 ± 0.181 ns/op SynchronizedBenchmark.viaSetterMethod str avgt 15 7.802 ± 0.193 ns/op SynchronizedBenchmark.viaMethodHandle_setterMethod str avgt 15 17.766 ± 0.551 ns/op SynchronizedBenchmark.viaMethodHandle_field str avgt 15 17.939 ± 0.290 ns/op SynchronizedBenchmark.viaReflection_field str avgt 15 157.453 ± 2.678 ns/op SynchronizedBenchmark.viaReflection_setterMethod str avgt 15 157.346 ± 2.049 ns/op SynchronizedBenchmark.viaReflection_setterMethod_synchronized str avgt 15 542.966 ± 19.825 ns/op SynchronizedBenchmark.viaReflection_field_synchronized str avgt 15 771.381 ± 47.922 ns/op I added in using MethodHandles in the tests, just to show the difference. When Sling stops supporting Java 8, we can move to MethodHandle which has significantly better performance characteristics. Unfortunately, we can't make that move until we are on at least Java 9, so that we can use MethodHandles.privateLookupIn(..) [2]. Regardless, you can see the significant difference between synchronized vs non-synchronized: synchronizing is 3-8x slower than not, when under contention with multiple threads. [1]: https://gist.github.com/paul-bjorkstrand/f3bb154665e7d2168b4656eb7b794496 [2]: https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/MethodHandles.html#privateLookupIn-java.lang.Class-java.lang.invoke.MethodHandles.Lookup- -Paul On Sat, Dec 4, 2021 at 9:48 AM Paul Bjorkstrand <paul.bjorkstr...@gmail.com> wrote: > In light of discussion on a recently merged PR [1], I raised the question > 'do we need to "reset" the accessible flag for reflective injectables > (fields, methods, constructors)?' > > The sole purpose of synchronized around the injection logic (including > constructor instantiation) seems to surround properly resetting the > accessible flag on the reflection element, and ensuring the element is > accessible during the injection. > > It is likely worth the effort to determine if resetting that flag, and > thus the accompanying synchronized, is really needed. As a reference, I > searched Apache Felix for uses of setAccessible, as it also needs to be > able to inject into non-public elements [2]. After browsing a handful of > the non-test classes, I found none that do anything more than > setAccessible(true). This leads me to believe that we are doing extra work > in Sling Models that isn't really necessary. > > Removing synchronized could have a dramatic, positive impact on > performance, especially for situations where there may be high contention > (e.g. heavy model/submodel reuse, sharing a common super class, etc). If > those synchronizations were removed, overall throughput will likely > increase. > > Refs: > [1]: > https://github.com/apache/sling-org-apache-sling-models-impl/pull/11/files#r762409049 > [2]: https://github.com/apache/felix-dev/search?q=setAccessible > > -Paul >
