Performance/intervalmap

[bradNASA]

• Tickets addressed: none, afaik
• Review: By file ?
• Merge strategy: Merge (no squash)

Description

IntervalMap changed to use TreeSet instead of ArrayList for Segments to speed up set(), which is used to construct an IntervalMap from a list of Segments. Some SPHEREx scheduling code for merging SimulationResults is very slow because set() makes IntervalMap construction O(n^2). The use of TreeSet makes that O(n log n). I am not sure what other code may benefit from this.

Joel Courtney pointed out that the use of a list is more efficient in some cases, for example, for appending non-overlapping Segments. He thought it might make sense to keep the list and use a tree in some cases. There may be some better data structure that takes advantage of both, like an ordered list with binary search operations; maybe a skip list would, too.

There's a shortcut checking whether the input segments meet invariant conditions in order to ingest Segments without using set(), which handles overlapping.

There are changes to IntervalAlgebra for faster low-level interval queries.

Verification

Unit tests pass seemingly faster (maybe 3.25 instead of 3.75 minutess), but variance is high. I still need to verify it solves the problem in the SPHEREx model.

Documentation

None, afaik

Future work

Interval code elsewhere in Aerie may also need updating like this.

[bradNASA]

@mattdailis Any reason we shouldn't merge this?

[JoelCourtney]

I haven't looked at every change to IntervalMap in detail, but I do know that Pranav and I tested the bejeezus out of that class back in the day. It sounds theoretically sound and the tests pass. 👍

[bradNASA]

I haven't looked at every change to IntervalMap in detail, but I do know that Pranav and I tested the bejeezus out of that class back in the day. It sounds theoretically sound and the tests pass. 👍

Yeah, I was impressed with the detail of the test suite.

[dandelany]

Thanks @bradNASA , merging!