#22115 closed defect (fixed)
[PATCH][RFC] Extract methods from LatLon into ILatLon where they are generally applicable
Reported by: | taylor.smock | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 22.06 |
Component: | Core | Version: | |
Keywords: | performance | Cc: |
Description (last modified by )
The attached patch does the following:
- Moves
greatCircleDistance
andbearing
intoILatLon
fromLatLon
- Where possible, remove
getCoor()
calls and instead call the appropriate method directly - Deprecates the original methods (rather unfortunately, this does mean we will have deprecation warnings when compiling for a bit, as most of the remaining calls deal with EastNorth conversions)
- Where possible (when noticed), if something calls
Node#getCoor
and then directly passes the result into something that acceptsILatLon
, theNode
is now passed instead.
This does not move interpolate
and getCenter
, as both return a LatLon
.
For those reading the code, Node
extends INode
which extends ILatLon
.
This should decrease overall memory allocations significantly. One major win is in SearchCompiler#match
, where a Bounds#contains(LatLon)
is replaced by Bounds#contains(ILatLon)
, where the ILatLon
is the Node
(in a validation test of Mesa County, Colorado, the getCoor
call accounted for 284 MiB of 2.216 GiB memory allocations for SearchCompiler#Match
).
Note: I'm not sold on the @deprecation
annotations. I'd like to be able to remove the methods from LatLon
someday, but those methods are used a lot, so there will be a lot of breakage when we do remove them.
Profiling results (again, Mesa County Colorado nwr in "Mesa County, Colorado"
):
Node#getCoor
(using method back traces) went from 501 MiB to 97 MiB.SearchCompiler.InArea#Match
went from 2.216 GiB to 2 GiB, which is about right.
Attachments (1)
Change History (23)
by , 3 years ago
Attachment: | 22115.patch added |
---|
comment:1 by , 3 years ago
Description: | modified (diff) |
---|---|
Milestone: | → 22.06 |
Summary: | [PATCH] Extract methods from LatLon into ILatLon where they are generally applicable → [PATCH][RFC] Extract methods from LatLon into ILatLon where they are generally applicable |
comment:2 by , 3 years ago
Description: | modified (diff) |
---|
comment:3 by , 3 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:6 by , 3 years ago
see #22146 for a regression when old JOSM jar is used with new plugin jar. Not sure which other modules are involved.
comment:7 by , 3 years ago
Looks like I failed to update the plugin mainversion value for it. I'll see if I can revert r35978/osm for utilsplugin2. I believe I got the build.xml
for everything else updated.
follow-up: 10 comment:9 by , 3 years ago
I think you just have to revert the dist, then fix the problem and dist again?
comment:10 by , 3 years ago
Replying to GerdP:
I think you just have to revert the dist, then fix the problem and dist again?
Maybe? I'd have to ask stoecker/Don-vip. I'm going to go for the surefire way (AKA revert, build, commit dist, etc.)
follow-up: 19 comment:16 by , 3 years ago
Besides photoadjust, following plugins need to be checked, too:
- building_tools
- indoor_sweepline
- livegps
- pbf
- tracer
comment:19 by , 3 years ago
Replying to skyper:
- building_tools
Should be fine. The only real change was internal to the plugin (latlon2eastNorth
). EDIT: See r35756/osm
- indoor_sweepline
Same as buildings_tools. EDIT: Compiles with r18464. Should be fine.
- livegps
r35975/osm increased it to 18464. EDIT: Doesn't seem to be released via svn.
- pbf
Should be fine. It was getCoor
+ lat
/lon
calls.
- tracer
r35975/osm increased it to 18464. EDIT: Compiles with r18464. Should be fine.
I'll double check. It doesn't help that 18464 != 18494
.
comment:22 by , 3 years ago
Replying to skyper:
Thanks Taylor and sorry for the noise.
No worries. I thought I checked all of them using a for
loop to compile. I should have just increased the min JOSM version for all of them.
In 18494/josm: