inst: Add a way to get an event when the destruction of an instance actually occurs

[muraj]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 5.55556% with 17 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1674659). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/realm/inst_impl.cc	6.66%	14 Missing ⚠️
src/realm/mem_impl.cc	0.00%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #300   +/-   ##
=======================================
  Coverage        ?   26.63%           
=======================================
  Files           ?      188           
  Lines           ?    39022           
  Branches        ?    14222           
=======================================
  Hits            ?    10393           
  Misses          ?    27642           
  Partials        ?      987           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lightsighter]

Is there a reason to introduce a new method call and not just make the old API start returning an event? Overhead of triggering an event for an instance destruction should be very low.

[muraj]

Triggering should be low, but I worry about creating an event on destroy. I didn't want to inadvertently regress perf on a heavily utilized call. I could go with returning an event, but I don't know what to tell QA to run to verify this isn't an issue. Was hoping you might have some insight?

I still need to write tests for this change, so it's technically not done yet, but wanted a quick check to verify the concept.

[lightsighter]

I could go with returning an event, but I don't know what to tell QA to run to verify this isn't an issue.

Run the Legate performance test suite. It should be fairly representative of at least some interesting workloads.

Was hoping you might have some insight?

My intuition is that we should just always return an event. Even if the instance creation/deletion is on the critical path, the cost of triggering the event will be significantly lower than the cost of running through a memory allocator to make the instance (e.g. Realm's allocator or cudaMalloc, etc.). I don't expect to see any performance regression from returning an event all the time.

[muraj]

@lightsighter updated to always return an event. It'll not create a destroy event if wait_on is Event::NO_EVENT, and just return Event::NO_EVENT in that case, which is always triggered, as a small optimization. I can go deeper and only do this when the wait_on event is triggered, but I'm not sure it's worth it.

[lightsighter]

It'll not create a destroy event if wait_on is Event::NO_EVENT, and just return Event::NO_EVENT in that case, which is always triggered, as a small optimization. I can go deeper and only do this when the wait_on event is triggered, but I'm not sure it's worth it.

I think that is perfectly fine and compliant with the Realm API. Presumably though I'll still get an actual event back if I do a destroy with a NO_EVENT as a precondition if I issue the destroy on a remote node from where the instance lives and a message needs to be sent to the owner node to do the actual destroy right? The event will trigger after the message has made it to the owner node and been applied.

[muraj]

if I issue the destroy on a remote node

Ah, hm, no, I'll have to add that.