search

snazzy-d / sdc

The Snazzy D Compiler
MIT License
246 stars 55 forks source link

Add finalizer calls from collection cycle for large blocks #368

Closed schveiguy closed 1 month ago

schveiguy commented 3 months ago

This adds support for finalizing large blocks in SDC's garbage collector when a collection cycle is run.

deadalnix commented 3 months ago

This is doing many things at once. At least 3:

  1. Add finalization for dense allocations.
  2. Fix the issue where druntime and the GC do not communicate the range to scan for the stack properly.
  3. Address performance problems in __sd_gc_druntime_qalloc .

Ultimately, it'd be better if this was 3 PR instead of one. Any comment on any of these points will block the other 2, which isn't what we want. In addition some of this code could go into master, while the rest still rely ion a WIP on druntime side and likely shouldn't be merged in master right away.

But first and foremost, let's make sure we stick with the style of the project. tool/autoformat can help greatly there.

schveiguy commented 3 months ago

OK, I rebased on master, and rebased on druntimegc. But your druntimegc does not include the extern(C) changes to dmd/thread.d, so it looks like I'm redoing it here. But really, this is from master. I'm not sure what to do here.

I'll at least work on getting the other parts split out.

schveiguy commented 3 months ago

3. Address performance problems in __sd_gc_druntime_qalloc .

Hm... I don't think this is right. What I did was add back in the code that was bad, but just commented out. We don't know if we may need it later...

schveiguy commented 2 months ago

OK, I think this is ready to be looked at again.

schveiguy commented 2 months ago

Oh, I can probably do the sparse blocks too. Will work on that.

schveiguy commented 2 months ago

Alright, the finalizer support is all there. I still don't know why 32-byte blocks don't get collected correctly. Am investigating. But it has nothing to do (I think) with the finalizer changes.

Edit: figured out the 32-byte block thing -- it's a separate issue that has to do with the TLS cache for fast allocation being scanned.

schveiguy commented 2 months ago

updated. Still some remaining questions.

schveiguy commented 2 months ago

rebased, I need to test for performance and correctness.

schveiguy commented 2 months ago

Most things resolved. Just the one question of refactoring.

schveiguy commented 2 months ago

Just realizing, this should have tests. Where do you put library tests?

schveiguy commented 2 months ago

Test added, should be ready to go.

schveiguy commented 2 months ago

looks like the CI is trying to install some package that doesn't exist?