@kswang1029 to do performance test via e2e tests. check test elapsed time!

@acdo2002 please execute ICD test manually in a sequence (not in parallel) and compare elapsed time of each test to see if there is significant performance boost or degradation. I will do a similar one but from e2e perspective. @confluence these test results would provide a hint about performance impact hopefully.

@markccchiang @confluence Not sure if I am seeing a thing here. It appears to me that dev (with map) performs better than unordered_map branch. Basically all tests with unordered_map branch require more time to execute.

Hmm. In that case, I suggest that we move this to draft until I can dig further into the code and look at how we're using the maps in different components. It's possible that only some of the maps should be changed, none of them should be changed, or that some of our unordered maps should be ordered maps! Clearly it's not as simple as changing them across the board.

Hmm. In that case, I suggest that we move this to draft until I can dig further into the code and look at how we're using the maps in different components. It's possible that only some of the maps should be changed, none of them should be changed, or that some of our unordered maps should be ordered maps! Clearly it's not as simple as changing them across the board.

Any naive explanations why?

Any naive explanations why?

My short answer is: the way we're accessing the maps is (on average!) triggering cases where map is better more than the cases where map is worse. Some guesses:

• unexpected hash collisions (could be a bug in a hash function we've defined -- that's happened before!)
• something else to do with key hashing (e.g. we have a complicated key type and the hashing and/or equality comparison is expensive)
• we're performing one of the slow operations more often than I thought

But these are just guesses, and this requires further investigation. It's possible that only one of these maps is the source of the slowdown, and all the other maps do in fact benefit from being converted (but the one slow map is disproportionately affecting the outcome because it's accessed a lot more frequently).

(Edit: my best guess is that using the StatsRegionId struct as a key, with custom hashing and comparison functions, is expensive. I have an idea for testing this theory, but I need to think about how to implement it correctly.) This is only used in a very limited way, so it's probably not that. String keys may be expensive to hash, though, and we use those in a few places in the metadata code.

I've also noticed that some of the region code repeatedly creates and destroys local variables which are small maps, and this might be more expensive for unordered maps. But I also don't know if this is happening frequently enough to be an issue. Rather than continuing to guess, I will try to use a profiler. :)

I've also noticed that some of the region code repeatedly creates and destroys local variables which are small maps, and this might be more expensive for unordered maps. But I also don't know if this is happening frequently enough to be an issue. Rather than continuing to guess, I will try to use a profiler. :)

indeed, region e2e tests do have the performance degradation problem more significantly. All the e2e tests have a set of common step: load file list, load file info, and then load image or images.

Hmm. In that case, I suggest that we move this to draft until I can dig further into the code and look at how we're using the maps in different components. It's possible that only some of the maps should be changed, none of them should be changed, or that some of our unordered maps should be ordered maps! Clearly it's not as simple as changing them across the board.

Yes. Considering the current performance test results, we may switch between map and unordered_map one by one, and see which is better. This is trivial but time-consuming work -- so far there is no clear guide on which kind of map is better for us to use.

I did a quick test, here is the result.
the current dev
current performance regression tests (not much region related, big 35 GB fits, casa and 70GB hdf5):102.882 s, 100.716 s, 100.641 s, 100.945 s, 100.977 s
ICD-RxJS (small image size) region related tests:3.341 s, 3.442 s, 3.49 s, 3.465 s, 3.473 s

the unordered map branch:
current performance regression tests (not much region related, big 35 GB fits, casa and 70GB hdf5):101.107 s, 101.149 s
101.007 s, 100.931 s, 100.984 s => NO big difference
ICD-RxJS (small image size) region related tests:3.535 s, 3.48 s, 3.467 s, 3.498 s, 3.527 s => little bit slower, perhaps need the specific large region sizes test rather than bigger image size tests

NOTE: there is a regression failure happened on SET_REGION_ANNOTATION_EXPORT_IMPORT_CASA_PIXEL.test.ts
The exported region does not match to the expect
Expected: 163
Received: 157.71336364746094