[WIP] AVL tree + performance tweaking

[w8r]

• proceeding with work on adopting faster tree data structure
• Took in @rowanwins changes for union construction
• split the code for convenience
• trying to tweak the performance

[rowanwins]

Hi @w8r

I've been doing some scratching through this martinez implementation to see if it can provide anything useful

I wondered if this chunk in particular might help in the subdivide_segments function for unions given that we're experiencing slowness particuarly with our union operation...

My initial implementation of it is failing, I simply added the following but I think Im incorrectly interpreting the connector.add() as sortedEvents.push()

    if (operation === UNION && (event.point[0] > rightbound)) {
      while (eventQueue.length) {
        event = eventQueue.pop();
        if (!event.left) sortedEvents.push(event);
      }
      break;
    }

Will see if I come across anything else useful

[rowanwins]

Looking at this implementation it looks as though the creation of the polygon is done as new vertices are added, rather than at the end (which our connect_edges() currently appears to do...). Not sure if there are major gains here but it is quite different.

[rowanwins]

Also note there are some c++ files in here which you might find easier to read.

[rowanwins]

And another potential source for inspiration using php. This one looks relatively new and well organised.

[rowanwins]

Hi @w8r

Well I've made a very simple change but it looks like a made a big difference and martinez is now out-performing jsts for all operations. I think it could be taken a step further with the right knowledge of what's happening but it took me 1.5 hours just to add those 30 characters of code :)

[w8r]

Oh this is amazing, you cut off so much unnecessary loops! I totally didn't see that opportunity

[rowanwins]

interestingly the benchmark test doesn't appear to make as much difference as the browser, I wonder why that is...

[rowanwins]

It looks like the next hurdle might be something in the subdivide_segments

[w8r]

But this one I have mostly kicked out trying to minimize function calls. However it didn't give much gain

[DenisCarriere]

Simple operations like this won't have much performance gains in the grand scheme of things.

However, dropping for loops helps a lot!

[rowanwins]

yeah this was a bit of a long shot but I figured if it was a micro-optimisation that was being called a squillion times it might help, see https://jsperf.com/andcomparitor/1 (although the results are depenedent on what browser you use)

[w8r]

Now I see some problem in difference on asia.geojson against a polygon with hole (it crashes), but I didn't check the output yet

[DenisCarriere]

martinez is now out-performing jsts for all operations

👏 😄

[gap777]

@w8r we could really use these changes... any idea when you think they'll be ready to merge?

[rowanwins]

Hi @gap777

Unfortunantly it's still a little while away. Hopefully once I get the new Turf.js docs released in the next week I'll be able to do some digging into the remaining bugs.

[rowanwins]

Hi @w8r

I'm starting to look at this again, I'm determined to crack it one day!

You'll see I've added a new style of test which is designed to check each of the potential input geometries (polys, poly with hole, multipoly) against each of the operations. These test use super-simple geometries to try and debug and are clearly named so hopefully they will help us track down what is happening.

[w8r]

This is amazing, feel free to poke me, I am supposed to have more free time this week

[rowanwins]

Will try and get the suite roughly ready and then will report back on those which are throwing issues.

[rowanwins]

Ah thanks for that. Looking at it the result of that holecut difference looks incorrect, while the visual result shown on the map looks ok the actual result array is incorrectly nested, I'll do a quick dig now and see if I can work out why...

[w8r]

yes, nesting is the problem there

[rowanwins]

Think I've got it sussed, just running some more checks, stay tuned

[rowanwins]

Well that was good, I removed some lines that had always concerned me regarding the operation type. And I was also able to remove that final loop as well.

[rowanwins]

Bah sorry confused myself when I was checking others. I know why it's not working, its because it's marked as an interior ring. But if it's an exterior ring on a difference operation, but the subject box is within the clipping bbox, then it should be treated as a hole. I haven't worked out how to best test for it though and rectify it.

[rowanwins]

Have pushed a fix for that difference hourglass nesting bug , not sure how solid it is but the tests pass.

[w8r]

I guess everything is ok

[w8r]

I'll squash if you don't mind, don't see how the separate commits would help reconstructing this drama

[w8r]

#35 is still completely broken, but ok, it will be the next one https://codepen.io/w8r/pen/XVKNQw?editors=0010

[w8r]

and #12 will be broken after the merge

[w8r]

so it fixes #47,#45, #44, #43 (diff still broken), #17, #32

[gap777]

@w8r and @rowanwins - it sounded like you guys were headed towards a release soon. What's your intention in the near future?

[w8r]

thank you @rowanwins !

[w8r]

released as v0.3.0. I saw a couple of small problems with a tree and also how to improve the demo and releasing, that will go to the patches

[rowanwins]

That was epic @w8r !!!

[codeofsumit]

👏 👏 nice!