tomoyanonymous / mimium-rs

minimal musical medium- an infrastructural language for sound and music.
Mozilla Public License 2.0
9 stars 1 forks source link

Add simple reference counted GC for Closure. #54

Closed tomoyanonymous closed 2 months ago

tomoyanonymous commented 2 months ago

Continued from #50. Also close #10

Now, unused closures are automatically deallocated correctly These 2 integration tests now works with gc.

Also, GC unit tests were commented out but now re-enabled.

With this PR, upvalue newly holds a partial type information on runtime: a flag for detect whether it is closure or not.

This is because the heap memory for normal closed upvalues are managed with Rc<RefCell<Vec<RawVal>>>, but the type information is erased, At the same time, we can't do normal RAII solution because the actual closures are managed on the slotmap on the VM.

Therefore, closures are managed with their individual reference counter separately.

We use this reference counter and partial type information mostly on close_upvalues and drop_closure because the heap-allocated values are made(incrementing count) only on upvalue closing, and drop(subtracting count) happens only on the deallocation of parent closure.

Because this is reference counted GC, closures that refers to each other can not be deallocated but I leave it for now...

tomoyanonymous commented 2 months ago

Bench result: IDKW, but its getting faster...

Before

test tests::runtime::bench_multiosc10   ... bench:       3,117.25 ns/iter (+/- 176.88)
test tests::runtime::bench_multiosc15   ... bench:       4,684.74 ns/iter (+/- 491.45)
test tests::runtime::bench_multiosc5    ... bench:       1,587.83 ns/iter (+/- 90.00)
test tests::runtime::bench_multiosc7    ... bench:       2,225.87 ns/iter (+/- 83.58)

After

test tests::runtime::bench_multiosc10   ... bench:       2,904.32 ns/iter (+/- 149.47)
test tests::runtime::bench_multiosc15   ... bench:       4,354.90 ns/iter (+/- 371.32)
test tests::runtime::bench_multiosc5    ... bench:       1,487.63 ns/iter (+/- 83.62)
test tests::runtime::bench_multiosc7    ... bench:       2,065.94 ns/iter (+/- 145.85)
yutannihilation commented 2 months ago

Great progress!

Because this is reference counted GC, closures that refers to each other can not be deallocated but I leave it for now...

Does this refer to scheduler_counter_indirect.mmm? I'm wondering why the number of closures increases infinitely in this case.

After I saw this comment on https://github.com/tomoyanonymous/mimium-rs/pull/50#issuecomment-2348290997, I've been thinking if, even when the latter option is chosen, the first one might be needed anyway in order to distinguish the reference with and without @ for some temporal analysis; when there's only one reference with @, probably the scheduler should be responsible for dropping the closure after the time specified at @ has passed. (after some thinking, this is probably my misunderstanding.)

I will try to solve this in another PR, basically with either of 2 ways.

  • handle _mimium_scheduler_at as a special function
  • implement a straightforward garbage collection (mark&sweep or reference counting)
tomoyanonymous commented 2 months ago

I've been thinking if, even when the latter option is chosen, the first one might be needed anyway in order to distinguish the reference with and without @ for some temporal analysis; when there's only one reference with @, probably the scheduler should be responsible for dropping the closure after the time specified at @ has passed.

I tried to address that issue in 04c5909 but there was unnecessary refcount increment.

It was fixed now in 0a0d3f4

yutannihilation commented 2 months ago

Thanks, makes sense. After I wrote the comment, I found the implementation does what I expected... Anyway, congratulations on the milestone!