Cleanup stacks in pc group collection code #2461
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fingolfin
added
do not merge
fingolfin
force-pushed
the
mh/collector-stacks
branch
from
6c1410f
to
46cedfd
Compare
Codecov Report
@@ Coverage Diff @@
## master #2461 +/- ##
==========================================
+ Coverage 74.19% 74.19% +<.01%
==========================================
Files 484 484
Lines 245306 245311 +5
==========================================
+ Hits 181996 182011 +15
+ Misses 63310 63300 -10
|
fingolfin
changed the title
fingolfin
removed
the
do not merge
fingolfin
force-pushed
the
mh/collector-stacks
branch
from
46cedfd
to
4e818a0
Compare
fingolfin
force-pushed
the
mh/collector-stacks
branch
from
4e818a0
to
c866516
Compare
Otherwise, if a garbage collection happens after we assign a new bag to <bag>, but before calling InitGlobalBag on it, the new bag may be garbage collected, and we end up with a bad reference.
These stacks were created as T_PLIST_EMPTY, but then used as if they were T_STRING (but with the length field overwritten with arbitrary data), then were resized into T_STRING. Now they are T_DATOBJ, and we make sure not to overwrite the type slot.
... instead of using the stack objects in the pcp collector objects provided by the polycyclic package. This will make it possible to remove the latter in a future version of polycyclic, thus reducing the size of these objects considerably.
ChrisJefferson
approved these changes
May 18, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first commit is motivated by the Julia GC work: the classic pc group collected code in the kernel (ab)used T_PLIST_EMPTY resp. T_STRING objects to store various stacks, but overwrote the length slot with garbage in the process. We change it to use a T_DATOBJ, and take care not to overwrite its type slot.
The second commit adds in-kernel stacks for the "new" pc(p) group collector, used by the polycyclic package. Right now, every pcp group collector has its own set of stacks, resulting in collector objects which are several hundred kilobytes big. This is a crazy waste if you have many collectors around.
TODO: more tests; and in order to allow polycyclic to smoothly transition away from the stacks-in-collectors, we need to add a way for it to find out when it can do that. I guess I'll just add a constant
NO_STACKS_INSIDE_COLLECTORSor so, and then make a polycyclic release using it to disable the stack if it is set to true. And once that happened, we can think about removing the relevant constants likePC_WORD_EXPONENT_STACKfrom the kernel...