rts: Make StablePtr derefs thread-safe (#10296)
ClosedPublic

Authored by jme on Mar 21 2016, 9:10 PM.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jme updated this revision to Diff 7007.Mar 21 2016, 9:10 PM
jme retitled this revision from to rts: Make StablePtr derefs thread-safe (#10296).
jme updated this object.
jme edited the test plan for this revision. (Show Details)
jme added reviewers: ezyang, simonmar.
jme updated the Trac tickets for this revision.
simonmar requested changes to this revision.Mar 23 2016, 3:48 AM
simonmar edited edge metadata.

Firstly, good catch!

I like the idea of retaining the old table so that we can ensure deref remains lock-free. But I suggest a refinement: free the old table(s) during GC, when we know there are no concurrent derefs in progress.

This revision now requires changes to proceed.Mar 23 2016, 3:48 AM
jme updated this revision to Diff 7016.Mar 23 2016, 2:34 PM
jme edited edge metadata.

Free old versions of the stable pointer table during GC

jme added a comment.Mar 23 2016, 2:36 PM

Good idea to free during GC--definitely simpler than what I had in mind.

bgamari accepted this revision.Mar 24 2016, 7:56 AM
bgamari edited edge metadata.

Looks reasonable to me.

carter added a subscriber: carter.Mar 24 2016, 11:31 AM

Question: am I correct in understanding that stable pointers only get freed during GC, but that the table itself is not on the heap?

In that case would a more "slow" shrinkage policy make sense , kinda like how for those c++ data structures that 2x when they grow, but. /1.5x when usage halves to allow cheaper reinserts? I guess that would entail doing at most some constant ratio contraction , say / 1.5 x, but would (I imagine) have better worst case heavy stable pointer with a short life time application load, while at the same time provide the right space usage after a few gcs in the drop all the stable pointers case.

This design detail probably doesn't matter, but figured I'd mention it just in case continuous allocation shortlived stable pointers is an application pattern that is anticipated to occur.

jme added a comment.Mar 24 2016, 1:39 PM
In D2031#59608, @carter wrote:

Question: am I correct in understanding that stable pointers only get freed during GC, but that the table itself is not on the heap?

Yes, the table is not on the heap. A stable pointer in the table is only freed (i.e., put back on the stable pointer free list) by an explicit call to freeStablePtr.

In that case would a more "slow" shrinkage policy make sense , kinda like how for those c++ data structures that 2x when they grow, but. /1.5x when usage halves to allow cheaper reinserts?

Actually, the performance issue with an underutilized table involves GC; see Trac #10470. From the little I've seen though, the stable pointers in most programs seem to be fairly long-lived.

@simonmar, any last requests before this is merged?

bgamari requested changes to this revision.Mar 30 2016, 2:56 PM
bgamari edited edge metadata.

Could you rebase this @jme? Arcanist seems to be struggling to apply it.

This revision now requires changes to proceed.Mar 30 2016, 2:56 PM
jme added a comment.Mar 30 2016, 3:49 PM

@bgamari -- sure, no problem. Plus, I can now change my two ints to nats. It's been bothering me all week, but I didn't want to submit a new revision for such a tiny change.

jme updated this revision to Diff 7150.Mar 30 2016, 6:18 PM
jme edited edge metadata.
  • Rebased to latest HEAD
  • Changed two ints to nats
This revision was automatically updated to reflect the committed changes.