Hi Gerd,
WanMil wrote
* Can you please add some javadoc to the new methods? I know the old methods often do not have javadocs but that shouldn't be continued...
I totally agree. The attached correction contains comments. I am a little bit in trouble with this because the new methods and the corresponding flags in Coord are only meaningfull inside the existing removeShortArcsByMergingNodes() method of ElementSaver, but I think the performance improvement is worth enough to allow this rather bad coding style.
Did you try to use another storage mechanism within the removeShortArcs... method? I don't like storing the infos of the short arcs in *all* coords. Your patch reduces the memory footprint of the Coord object (great!). But I think information should be as local as possible.
* RuleIndex.getRulesForTag returns a BitSet that can be modified externally. This is no good style. I know that the BitSet is not modified externally so it's not a bug. But at least is should be documented in the javadoc that the returned BitSet *must not* be modified.
In fact, the method was wrong because it evtl. modified by mistake the BitSet in tagVals. This is corrected with the new patch which returns a newly created BitSet.
Great!
* I propose to use an ArrayList instead of HashSet in line 157 of RuleIndex
OK, changed. This part of the code is not performance critical, so I wanted to keep the changes small.
* RuleIndex line 175: The set.clear(i) method changes the BitSet in tagVals/tagnames directly. The unpatched mkgmap is working on a copy. Can you please check carefully if your version is correct?
Good catch, my version was definetly wrong. This is also corrected with the new patch.
The new patch adds one more small change: In removeShortArcsByMergingNodes() Coord.distance() is only called if we really have to calculate the length of the arc. If parameter remove-short-arcs is used without a value (or with remove-short-arcs=0), there is no need to calculate the distance, we just need to know if the two points are equal.
I also have one question: What is causing mkgmap to produce different results each time when it executes? This makes optimization very difficult because one cannot simply compare old and new result.
http://gis.638310.n2.nabble.com/file/n7129718/mkgmap_performance_3.patch mkgmap_performance_3.patch
Ciao, Gerd
All in all I see speed improvements using the patch. So after the Coord question is cleared I would like to commit that if nobody vetoes and you agree. Thanks! WanMil