Lazy reduces scalability of applications using Indriya #407
Description
Looking at a GeoTools benchmark lately I've found this stack trace reducing scalability, to the point that the application (map rendering) can only use 80% of CPU. Here is the trace involved, exported from the Yourkit profilter:
Back traces
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------+---------------+
| Reverse Call Tree | Time (ms) | Count |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------+---------------+
| +---tech.units.indriya.internal.function.Lazy.get() Lazy.java:71 | 24,602 100% | 3,246 100% |
| | | | |
| +---tech.units.indriya.unit.ProductUnit.hashCode() ProductUnit.java:298 | | |
| | | | |
| +---java.util.Arrays.hashCode(Object[]) Arrays.java:4685 | | |
| | | | |
| +---java.util.Objects.hash(Object[]) Objects.java:146 | | |
| | | | |
| +---tech.units.indriya.unit.AlternateUnit.hashCode() AlternateUnit.java:146 | | |
| | | | |
| +---java.util.Arrays.hashCode(Object[]) Arrays.java:4685 | | |
| | | | |
| +---java.util.Objects.hash(Object[]) Objects.java:146 | | |
| | | | |
| +---tech.units.indriya.unit.TransformedUnit.hashCode() TransformedUnit.java:220 | | |
| | | | |
| +---org.geotools.referencing.cs.DefaultCoordinateSystemAxis.hashCode() DefaultCoordinateSystemAxis.java:1319 | | |
| | | | |
| +---org.geotools.referencing.cs.AbstractCS.hashCode() AbstractCS.java:640 | | |
| | | | |
| +---org.geotools.referencing.crs.AbstractCRS.hashCode() AbstractCRS.java:165 | | |
| | | | |
| +---org.geotools.referencing.crs.AbstractSingleCRS.hashCode() AbstractSingleCRS.java:164 | | |
| | | | |
| +---org.geotools.referencing.crs.DefaultGeographicCRS.hashCode() DefaultGeographicCRS.java:212 | | |
| | | | |
| +---org.geotools.referencing.operation.BufferedCoordinateOperationFactory$CRSPair.<init>(CoordinateReferenceSystem, CoordinateReferenceSystem) BufferedCoordinateOperationFactory.java:70 | 22,255 90% | 2,953 91% |
| | | | | |
| | +---org.geotools.referencing.operation.BufferedCoordinateOperationFactory.createOperation(CoordinateReferenceSystem, CoordinateReferenceSystem) BufferedCoordinateOperationFactory.java:229 | | |
| | | | | |
| | +---org.geotools.geometry.jts.ReferencedEnvelope.transform(CoordinateReferenceSystem, boolean, int) ReferencedEnvelope.java:739 | 11,439 46% | 1,462 45% |
| | | | | | |
| | | +---org.geotools.geometry.jts.ReferencedEnvelope.transform(CoordinateReferenceSystem, boolean) ReferencedEnvelope.java:684 | | |
| | | | | | |
| | | +---org.geotools.renderer.crs.ProjectionHandler.preProcess(Geometry) ProjectionHandler.java:659 | | |
| | | | | | |
| | | +---org.geotools.renderer.lite.StreamingRenderer$RenderableFeature.getTransformedShape(Geometry, SymbolizerAssociation, boolean) StreamingRenderer.java:3586 | | |
| | | | | | |
| | | +---org.geotools.renderer.lite.StreamingRenderer$RenderableFeature.getShape(Symbolizer, AffineTransform, Geometry, boolean) StreamingRenderer.java:3544 | | |
| | | | | | |
| | | +---org.geotools.renderer.lite.StreamingRenderer$RenderableFeature.getShape(Symbolizer, AffineTransform) StreamingRenderer.java:3450 | | |
| | | | | |
| | +---org.geotools.referencing.CRS.findMathTransform(CoordinateReferenceSystem, CoordinateReferenceSystem, boolean) CRS.java:1329 | 10,815 44% | 1,491 46% |
| | | | | |
| | +---org.geotools.filter.function.NorthFix.evaluate(Object) NorthFix.java:143 | 10,025 41% | 1,363 42% |
| | | | | |
| | +---org.geotools.filter.function.NorthFix.evaluate(Object) NorthFix.java:151 | 790 3% | 128 4% |
| | | | |
| +---org.geotools.referencing.crs.AbstractDerivedCRS.hashCode() AbstractDerivedCRS.java:351 | 2,347 10% | 293 9% |
| | | | |
| +---org.geotools.referencing.crs.DefaultProjectedCRS.hashCode() DefaultProjectedCRS.java:242 | | |
| | | | |
| +---org.geotools.referencing.operation.BufferedCoordinateOperationFactory$CRSPair.<init>(CoordinateReferenceSystem, CoordinateReferenceSystem) BufferedCoordinateOperationFactory.java:70 | | |
| | | | |
| +---org.geotools.referencing.operation.BufferedCoordinateOperationFactory.createOperation(CoordinateReferenceSystem, CoordinateReferenceSystem) BufferedCoordinateOperationFactory.java:229 | | |
| | | | |
| +---org.geotools.referencing.CRS.findMathTransform(CoordinateReferenceSystem, CoordinateReferenceSystem, boolean) CRS.java:1329 | | |
| | | | |
| +---org.geotools.filter.function.NorthFix.evaluate(Object) NorthFix.java:151 | 1,443 6% | 197 6% |
| | | | |
| +---org.geotools.filter.function.NorthFix.evaluate(Object) NorthFix.java:143 | 903 4% | 96 3% |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------+---------------+
As far as I can see, the computation of ProductUnit hash code is considered expensive, and is thus memoized, but in a lazy way, using the Lazy class. The scalability issue comes from Lazy.get(), which adopts a synchronized block around the entire lazy loading:
I'm wondering, is that necessary? Or would it be ok to just allow the hash code to be computed eventually multiple times in parallel, but let the check for an already computed value free of synchronization? It may not be a general solution for all cases where Lazy is used, but seems to be useful for an operation that (should) lacks side effects and it's not massively expensive, like the computation of a hash code.