replace regex with indexOf

[karussell]

For #7. Not sure how to do performance tests in kotlin. Is there a native alternative for JMH?

Update: although this improves the import for my smaller test PBF it does not seem to improve the memory problems for planet.
Update: was a build problem

[westnordost]

Obligatory question: Did you test it? I.e. run the unit tests?

---

As mentioned earlier, the library does a lot of copying of lists, maps and sets where oftentimes it doesn't really have to, it was just simpler to write the code that way. E.g. keys.filter { it.startsWith("$key:") } creates a new set.

Now, I have recently read that copying data is one of those things modern CPUs are really good at, so oftentimes it doesn't really matter. However, maybe Java is different (because all collections are on the heap, not on the stack).
So, to traverse all elements on a collection lazily, you can use Sequences. Sequences are pretty much the same thing as Java Streams.

If using these at some places actually has an impact, I'd be curious to know. Might as well have a negative impact due to the added indirection, don't know 🤷 .

[karussell]

Obligatory question: Did you test it? I.e. run the unit tests?

Yes :)

Now, I have recently read that copying data is one of those things modern CPUs are really good at

Yeah, normally it is no problem and often the JVM can handle smaller frequently created garbage. But the GraphHopper import is delicate and it might not be the same (as a method is called millions of times). Will profile it again after this change. But the build makes me headaches as sometimes it worked to push it to the local maven repo and sometimes not.

[westnordost]

But the build makes me headaches as sometimes it worked to push it to the local maven repo and sometimes not.

Hm. Maybe the multiplatform setup is delicate... Is there an update for the multiplatform maven plugin?

[westnordost]

Also, I am not sure if publishing to local maven works if a local version with the same version number already exists. Maybe you need to delete the one you already published just then.

[westnordost]

So from my point of view, this could be merged now. Or do you want to also try out asSequence to not create unnecessary new lists/sets/maps in this same PR first?

[karussell]

Sure, feel free to merge it :) (I can't :) )

If you like or want it might be beneficial to setup the benchmark mentioned in #7 before merging, but this is up to you.

Or do you want to also try out asSequence to not create unnecessary new lists/sets/maps in this same PR first?

No, I think it is fine for me to approach this in smaller steps.

Obligatory question: Did you test it? I.e. run the unit tests?

Maybe you activate github actions to run the tests automatically?

[westnordost]

Maybe you activate github actions to run the tests automatically?

It would be prudent to do so... but I am too lazy to do this now. I'd have to read up a bit about github actions first. A PR that enables that however would be welcome :-)

[karussell]

btw: feel free to copy stuff from us https://github.com/graphhopper/graphhopper/blob/master/.github/workflows/build.yml (the node cache is probably not necessary)