Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy reduces scalability of applications using Indriya #407

Open
aaime opened this issue Dec 6, 2023 · 1 comment
Open

Lazy reduces scalability of applications using Indriya #407

aaime opened this issue Dec 6, 2023 · 1 comment

Comments

@aaime
Copy link

aaime commented Dec 6, 2023

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:

https://github.com/unitsofmeasurement/indriya/blob/master/src/main/java/tech/units/indriya/internal/function/Lazy.java#L71

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.

@andi-huber
Copy link
Member

I believe you cannot shortcut this sort of synchronization necessary here. But perhaps you can provide a code snippet to clarify what you have in mind and convince me otherwise.

Alternatively, perhaps you can do some performance tweaking to your org.geotools.referencing.cs.DefaultCoordinateSystemAxis.hashCode() such that it does not depend on the underlying indriya object's hash codes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants