WIP: enable GC sooner when loading precompiled modules#21185
WIP: enable GC sooner when loading precompiled modules#21185JeffBezanson wants to merge 1 commit intomasterfrom
Conversation
|
Dunno if this affects any of Nanosoldier's benchmarks, but... @nanosoldier |
|
I thought @vtjnash has something that doesn't require creating the array eagerly. (It's almost the same format as gc frames...) Why can't that be added to |
| jl_gc_enable(en); | ||
|
|
||
| jl_insert_methods(&external_methods); // hook up methods of external generic functions (needs to be after recache types) | ||
| jl_recache_other(); // make all of the other objects identities correct (needs to be after insert methods) |
There was a problem hiding this comment.
After this step, some new objects (from the flagref list) may be only accessible from old objects. I think we'll either need to update the flagref to track the parent so we can add the gc_wb in here, or prevent promotion.
There was a problem hiding this comment.
This call also re-writes the contents of the external_methods linked list (which is why it isn't stored in an array in the first place, since we need to ensure that the memory doesn't move during push). A better alternative strategy here is perhaps to build the deserialization state directly into extmeths. We probably just will need to pre-allocate the length (probably by calculating it during the serialization step – perhaps just collect them into an array and serializing that would work).
There was a problem hiding this comment.
Ah, yeah, I now remember that was the issue....
| } | ||
| for (i=0; i < backref_list.len; i++) { | ||
| jl_gc_push_root(ptls, (jl_value_t*)backref_list.items[i]); | ||
| } |
There was a problem hiding this comment.
we should never revisit the backref list after this point, so there's no need to root it
| if (loc) | ||
| jl_gc_push_root(ptls, *loc); | ||
| i += 2; | ||
| } |
There was a problem hiding this comment.
the flagref_list should only ever point to things that are part of the referenced elsewhere, so there's no need to root it
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
part of #20671
Loading the module still takes a long time, but now it actually completes instead of using a huge amount of memory (and, on my laptop at least, getting OOMkilled).