mps
mps copied to clipboard
Eliminate unnecessary write barrier on white segments
Eliminating the unnecessary write barrier on white segments in order to improve performance.
Closes #130 .
There is an existing review for this branch from @gareth-rees at in "Re: Unprotecting white set branch is ready for review" (e-mail) and a response is required.
There is an existing review for this branch from @gareth-rees at in "Re: Unprotecting white set branch is ready for review" (e-mail) and a response is required.
@gareth-rees wrote:
- The comment says, "We don't need to maintain the remembered set on white segments” but I am not confident that I understand why. I guess the reason must be that white AMC segments never get greyened (and so never need to be considered for scanning), but how do we know that is the case?
AMC is mostly-copying, so any objects not pinned down my ambiguous references are moved to new (grey or black) segments, and the memory occupied by the old copies is recycled. The old copies are never scanned.
If we implement multiple traces, then copes that appear old for one trace might be scanned by another, and we'll have to measure the tradeoff.
I ought to be able to refer you to design.mps.poolamc but #128 .
Clarified in 26152c3ea7af55151ca9dd34c1c2bd3d76162698 .
There is an existing review for this branch from @gareth-rees at in "Re: Unprotecting white set branch is ready for review" (e-mail) and a response is required.
@gareth-rees wrote:
- The comment goes on to say, "Don't do this for nailed segments, because they go on the grey list and the summary can be used to skip them later” which appears to contradict the previous sentence. So white AMC segments do get greyened if they are pinned.
Yes, but by the time the commented code is reached this has already happened, because all ambiguous scanning that might pin a segment happens before any exact scanning. The comment ought to make this clear.
Clarified and added assertion in 26152c3ea7af55151ca9dd34c1c2bd3d76162698 .
There is an existing review for this branch from @gareth-rees at in "Re: Unprotecting white set branch is ready for review" (e-mail) and a response is required.
@gareth-rees wrote:
- The comment says, “Don't do this for nailed segments” but I don’t see how this is achieved. It looks to me as though we could reach this block of code for a nailed segment if we get an exact fix to an object that was already nailed via a previous ambiguous fix.
The test at https://github.com/Ravenbrook/mps/blob/b763a6a0ce7c938df799b2b112a93bc31b7bffbd/code/poolamc.c#L1575 means that the code is not reached when fixing an ambiguous reference.
However you are correct that an exact fix to a nailed segment will reach this code and cause the remembered set to be forgotten. That's a mistake and I will correct it. This would do it:
if (SegRankSet(seg) != RankSetEMPTY && SegNailed(seg) == TraceSetEMPTY)
SegSetSummary(seg, RefSetUNIV);
It may be the case that we perform better by forgetting the remembered set in this case, and we should measure both cases.
Fixed in 26152c3ea7af55151ca9dd34c1c2bd3d76162698 .
There is an existing review for this branch from @gareth-rees at in "Re: Unprotecting white set branch is ready for review" (e-mail) and a response is required.
@gareth-rees wrote:
- The line
if (SegRankSet(seg) != RankSetEMPTY)
should have a comment mentioning AMCZ (as with other tests of this form).
Totally! Will fix.
Fixed in 26152c3ea7af55151ca9dd34c1c2bd3d76162698 .
Waiting for performance evidence from @thejayps