De-duplicate vertex labels to save memory

[leonardehrenfried]

Summary

Each vertex in the OTP graph has a materialized label and the vast majority of vertices are of type OsmVertex which have the duplicate label prefix of "osm:node:". This prefix is repeated millions of times in a large graph.

In order to cut down on the duplication this PR implements the following:

• convert label from a string to an interface so there can be multiple implementations
• use just the node id as the vertex label for OSM vertices
• make getLabel abstract in order to simplify the inheritance chain

Memory savings

The speed test suggests that this saves about 5-8% of memory. For example the Norwegian graph in the speed test went from 3.58 to 3.4 GB of memory.

Test removed

The test GeometryProcessorTest has been disabled for years and even if we could make it work, it is already covered by other tests. It would have needed quite a bit of refactoring to get it to compile again so I decided to delete it.

[codecov]

Codecov Report

Patch coverage: 74.10% and no project coverage change.

Comparison is base (74025a3) 65.64% compared to head (1452a51) 65.64%.

Additional details and impacted files

@@            Coverage Diff             @@
##             dev-2.x    #5223   +/-   ##
==========================================
  Coverage      65.64%   65.64%           
- Complexity     14712    14724   +12     
==========================================
  Files           1766     1769    +3     
  Lines          68580    68601   +21     
  Branches        7291     7290    -1     
==========================================
+ Hits           45017    45036   +19     
  Misses         21064    21064           
- Partials        2499     2501    +2     

Impacted Files	Coverage Δ
...opentripplanner/ext/geocoder/GeocoderResource.java	0.00% <0.00%> (ø)
.../org/opentripplanner/ext/geocoder/LuceneIndex.java	68.57% <0.00%> (ø)
...builder/module/AddTransitModelEntitiesToGraph.java	57.22% <0.00%> (-0.34%)	⬇️
...raph_builder/module/islandpruning/GraphIsland.java	3.84% <0.00%> (-0.16%)	⬇️
...builder/module/islandpruning/PrunedStopIsland.java	0.00% <0.00%> (ø)
...street/model/vertex/OsmBoardingLocationVertex.java	100.00% <ø> (ø)
...street/model/vertex/TransitBoardingAreaVertex.java	0.00% <0.00%> (ø)
.../street/model/vertex/TransitPathwayNodeVertex.java	0.00% <0.00%> (ø)
...rg/opentripplanner/visualizer/GraphVisualizer.java	0.00% <0.00%> (ø)
...aph_builder/module/islandpruning/PruneIslands.java	77.54% <50.00%> (ø)
... and 24 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[vesameskanen]

Very good improvement!

Using the new label type sometimes requires more verbose code than when using plain strings. Would it make sense to add a couple of simple helpers?

• In Vertex base class, a new method getLabelString()
• in Graph class, another vertex getter: getVertex(string label)

[leonardehrenfried]

* In Vertex base class, a new method getLabelString()

This one I find a good idea and will implement it.

* in Graph class, another vertex getter: getVertex(string label)

I'm not so sure about that one since it would have to know about all label formats and be able to parse them. What use case do you have in mind?

[vesameskanen]

The use case is a very common (especially in tests) expression like:

graph.getVertex(VertexLabel.string("shilshole_22nd"))

It would then appear as :

graph.getVertex("shilshole_22nd")

So the overloaded getVertex version would assume that the label is of type VertexLabel.string. Maybe this is a bit vague idea, as one label type gets a special treatment.

[leonardehrenfried]

Ok, that makes sense.