Refactoring code that is specific to stock GC write barriers

[udesou]

This PR moves some code responsible for implementing write barriers around: the write barrier functions are lifted from julia.h into the new gc-wb-*.h files.

[vchuravy]

Do we know yet how the write barriers for mmtk will look like?

The GCs share a tag layout?

I would prefer doing this code movement when we have a functional implementaion and not a "TODO implement these". That would also include tests for the new lowering in https://github.com/JuliaLang/julia/tree/master/test/llvmpasses

[udesou]

Do we know yet how the write barriers for mmtk will look like?
I would prefer doing this code movement when we have a functional implementaion and not a "TODO implement these". That would also include tests for the new lowering in https://github.com/JuliaLang/julia/tree/master/test/llvmpasses

I agree that it's not ideal to have only the stubs for the MMTk implementation. This branch currently has the final implementation of sticky working: https://github.com/udesou/julia/tree/upstream-ready/sticky.
But what I've been doing is to try to create smaller PRs that encapsulate these refactorings before adding the code that actually implements MMTk's sticky (generational) collector. As for the tests, that's a great reminder. I think we should definitely add tests, but perhaps it makes more sense to add them in a separate PR (the one in which we add sticky immix?)

The GCs share a tag layout?

No, the log bit is currently implemented as part of a side metadata maintained by MMTk. We can potentially look into reusing Julia's tag layout but I think the code movement in this PR is actually orthogonal to it. Any other GC that we'd eventually plug in with a different layout would have to reimplement the barriers accordingly. So I think it makes sense that the code for implementing and lowering the write barriers should live in each gc-specific file.

[vchuravy]

No, the log bit is currently implemented as part of a side metadata maintained by MMTk.

Huh, okay. That means we will need quite some more code-modifications in areas where we are currently directly modifying the GC bits?

But what I've been doing is to try to create smaller PRs that encapsulate these refactorings

I love small refactorings, but they must make the code clearer on their own. Currently, without knowing the other side of this code, it's difficult to tell if this refactoring is beneficial or if a different strategy would be better.

As for the tests, that's a great reminder. I think we should definitely add tests, but perhaps it makes more sense to add them in a separate PR (the one in which we add sticky immix?)

It's good to start with tests for the code as is. Then you can test everything and show that once you actually add the implementation what the emitted code will look like.

[d-netto]

The general idea here looks fine to me (have some minor comments which I will post shortly).

I believe that we proposed this refactor in the GC interface document already -- https://docs.google.com/document/d/1v0jtSrIpdEDNOxj5S9g1jPqSpuAkNWhr_T8ToFC9RLI/edit?usp=drivesdk.

The pattern we will follow to ensure that we inline these short functions in the
runtime is to conditionally #include in src/julia.h a header containing the write-barrier
implementation of Julia's stock GC or the implementation of the third-party GC.

[d-netto]

I love small refactorings, but they must make the code clearer on their own. Currently, without knowing the other side of this code, it's difficult to tell if this refactoring is beneficial or if a different strategy would be better.

Could you clarify these concerns about code clarity @vchuravy?

Asking because I'd expect the code structure to remain fairly similar after we merge the generational MMTk and implement proper write barriers.

I.e. I think the call-sites of these functions will remain the same, though we will change some of them (e.g. jl_gc_wb) from no-ops to something actually functional.

[d-netto]

I.e. I think the call-sites of these functions will remain the same, though we will change some of them (e.g. jl_gc_wb) from no-ops to something actually functional.

If that's the kind of refactor we will do, then I'm not sure how much more clarity we will be able to get by squashing this into a larger PR that introduces the generational MMTk.

[udesou]

I've removed the llvm-specific code, and left the refactoring of the jl_gc_wb* write barriers only. I'll leave the other changes to the PR that we add sticky so we have a better idea of what the code for MMTk will look like.

[d-netto]

The MMTk build seems to be segfaulting.

[udesou]

The MMTk build seems to be segfaulting.

The PR #57253 has changed the layout for _jl_module_t and added a new root. I'm updating the binding to propagate that change and fix the build for MMTk.

[d-netto]

LGTM. Perhaps rewrite the comments at the top of the WB header files

[oscardssmith]

The OOMs on 32 bit architectures are a bit concerning.

[d-netto]

The OOMs on 32 bit architectures are a bit concerning.

I'm skeptical they are related to this PR (which only moves a few functions to new files).

Let's rebase to rerun CI and see.

[KristofferC]

The OOMs on 32 bit architectures are a bit concerning.

I see these all the time