Incremental boehm support #958
Conversation
…nges are in there
|
Some relevant diffs in bdwgc repo: Current bdwgc 7.4 vs unity changes: bdwgc master vs unity changes (what I want to update to): |
|
Unity branch with a mono builds.7z built from this branch: https://katana.bf.unity3d.com/projects/Unity/builders?unity_branch=platform/foundation/mono-boehm-testing |
|
Corresponding PR for il2cpp: https://gitlab.internal.unity3d.com/vm/il2cpp/merge_requests/525 Since we want to keep bdwgc in sync between mono and il2cpp, we should probably consider the criteria for landing the two PRs together. |
| @@ -1227,8 +1227,12 @@ mono_domain_free (MonoDomain *domain, gboolean force) | |||
|
|
|||
| domain->setup = NULL; | |||
|
|
|||
| #if !HAVE_BDWGC_GC | |||
There was a problem hiding this comment.
I am asking upstream Mono about this issue.
mono/metadata/object.c
Outdated
| /* This is not needed by sgen, as it does not seem | ||
| + to need write barriers for uncollectable objects (like the vtables storing static | ||
| + fields), but it is needed for incremental boehm. */ | ||
| mono_gc_wbarrier_generic_nostore(t); |
There was a problem hiding this comment.
Let me look into this. Perhaps we should always be scanning the uncollectable regions of memory in Boehm but we are not.
There was a problem hiding this comment.
I guess we could, if we wanted to match sgen's apparent behavior here. But I don't see a reason to do that. It seems more efficient to rescan the uncollectable objects only when they have changed, like everything else.
There was a problem hiding this comment.
My main worry is making sure we don't miss any new places these are needed or any we didn't hit yet.
There was a problem hiding this comment.
So, I discovered most of these in Unity and mono through my test coverage. I have a mode where I run patch the GC functions to perform tracking of memory and then verify all references match set write barriers at the end of the run. I then made sure that RuntimeTests would pass without any unaccounted references using this setup. I am currently going through the same process for il2cpp.
The patching of GC functions is done at runtime by injecting a jmp instruction at the beginning of the functions. That way I can add the patching code to our development builds, and enable it through some environment variable. That should make it relatively easy to add configs to katana to run these tests, as is should be possible to do this by just adding a new katana config for each test suite we want to run with write barrier verification, without requiring custom builds of Unity and mono to enable that. (At least that's the idea, some open questions remain)
mono/metadata/boehm-gc.c
Outdated
| @@ -634,6 +647,7 @@ mono_gc_weak_link_add (void **link_addr, MonoObject *obj, gboolean track) | |||
| { | |||
| /* libgc requires that we use HIDE_POINTER... */ | |||
| *link_addr = (void*)HIDE_POINTER (obj); | |||
| GC_dirty(link_addr); | |||
There was a problem hiding this comment.
Nit, mono style is space between method name and parenthesis.
# Conflicts: # external/bdwgc
This PR:
-Updates Boehm GC subrepo to latest unity-master
-Implements write barriers needed for Boehm incremental GC in MANUAL_VDB mode
-Allows enabling incremental Boehm via environment variables
The write barriers should have no effect if incremental boehm is not enabled, so there should not be any risk from this. However, I see a risk in updating Boehm to the latest upstream (and not to an official release version). Not sure what kind of testing we should do to reduce such risk?