Closed milianw closed 3 years ago
Hi Millian!
Many thanks for this!
The only issue I see is that the operator+
for measurement data now will do the extra unnecessary ObjectExists(objectID)
check for every object in the two data sets (by replacing out.AddObjectID_()
with out.AddObjectID()
). But that function is unnecessarily expensive anyway, I think it needs to be rewritten to use AddObjectIDs()
instead.
Hey Cris,
many thanks for merging. I agree that operator+
is looking quite inefficient - porting it to AddObjectIDs
would help a lot. It should also try to reserve the final (estimated) size in the object map and then bulk-merge those too. We aren't using that code path, so I was reluctant to do any invasive changes there...
Yes, indeed. I just pushed a commit improving operator+
in exactly the way you describe. :) 26c7a052bae4115a34a29d148159e45d98d40bba
Hey Cris!
This merge request fixes two performance issues that I stumbled over while profiling our QiTissue cell feature measurement code. Both patches are pretty small and simple, yet have quite a positive impact on our code base, so I hope we can merge this!
So far I only ran our own benchmarks, are there any within diplib itself that I should run and/or expand to get this in?
Cheers