Change policy on root removal

digitalinfinity · digitalinfinity · commit 635cd8509118 · 2017-01-11T14:26:19.000-08:00
Currently, if RootRelease is called on a pinned object, we indicate that we
have a candidate for an exhaustive GC. However, we don't do this for other
sources of root removal- this change expands the RootRelease behavior to
all cases of explicit root unpinning.
diff --git a/lib/Common/Memory/Recycler.cpp b/lib/Common/Memory/Recycler.cpp
@@ -702,9 +702,9 @@ Recycler::RootRelease(void* obj, uint *count)
         RECYCLER_PERF_COUNTER_DEC(PinnedObject);
     }
 
-    // Not a real collection. This doesn't activate GC.
-    // This tell the GC that we have an exhaustive candidate, and should trigger
-    // another GC if there is an exhaustive GC going on.
+    // Any time a root is removed during a GC, it indicates that an exhaustive
+    // collection is likely going to have work to do so trigger an exhaustive
+    // candidate GC to indicate this fact
     this->CollectNow<CollectExhaustiveCandidate>();
 }
 
@@ -7717,6 +7717,11 @@ Recycler::DeleteGuestArena(ArenaAllocator * arenaAllocator)
     {
         guestArenaList.RemoveElement(&HeapAllocator::Instance, guestArenaAllocator);
     }
+
+    // Any time a root is removed during a GC, it indicates that an exhaustive
+    // collection is likely going to have work to do so trigger an exhaustive
+    // candidate GC to indicate this fact
+    this->CollectNow<CollectExhaustiveCandidate>();
 }
 
 #ifdef LEAK_REPORT
diff --git a/lib/Common/Memory/Recycler.h b/lib/Common/Memory/Recycler.h
@@ -1247,11 +1247,21 @@ class Recycler
     void UnregisterExternalGuestArena(ArenaData* guestArena)
     {
         externalGuestArenaList.Remove(&NoThrowHeapAllocator::Instance, guestArena);
+
+        // Any time a root is removed during a GC, it indicates that an exhaustive
+        // collection is likely going to have work to do so trigger an exhaustive
+        // candidate GC to indicate this fact
+        this->CollectNow<CollectExhaustiveCandidate>();
     }
 
     void UnregisterExternalGuestArena(ArenaData** guestArena)
     {
         externalGuestArenaList.RemoveElement(&NoThrowHeapAllocator::Instance, guestArena);
+
+        // Any time a root is removed during a GC, it indicates that an exhaustive
+        // collection is likely going to have work to do so trigger an exhaustive
+        // candidate GC to indicate this fact
+        this->CollectNow<CollectExhaustiveCandidate>();
     }
 
 #ifdef RECYCLER_TEST_SUPPORT
diff --git a/test/typedarray/rlexe.xml b/test/typedarray/rlexe.xml
@@ -314,14 +314,13 @@ Below test fails with difference in space. Investigate the cause and re-enable t
       <compile-flags>-mic:1 -off:simplejit -off:JITLoopBody -mmoc:0</compile-flags>
     </default>
   </test>
-  <!--Disabling this test case until the bug 9706251 is fixed
   <test>
     <default>
       <files>typedarray_bugfixes.js</files>
       <compile-flags>-Off:Deferparse -args summary -endargs</compile-flags>
       <tags>BugFix</tags>
     </default>
-  </test>-->
+  </test>
   <test>
     <default>
       <files>bug_OS_6911900.js</files>

Original file line number	Diff line number	Diff line change
`@@ -702,9 +702,9 @@ Recycler::RootRelease(void* obj, uint *count)`
`702`	`702`	`RECYCLER_PERF_COUNTER_DEC(PinnedObject);`
`703`	`703`	`}`
`704`	`704`
`705`		`- // Not a real collection. This doesn't activate GC.`
`706`		`- // This tell the GC that we have an exhaustive candidate, and should trigger`
`707`		`- // another GC if there is an exhaustive GC going on.`
	`705`	`+ // Any time a root is removed during a GC, it indicates that an exhaustive`
	`706`	`+ // collection is likely going to have work to do so trigger an exhaustive`
	`707`	`+ // candidate GC to indicate this fact`
`708`	`708`	`this->CollectNow<CollectExhaustiveCandidate>();`
`709`	`709`	`}`
`710`	`710`
`@@ -7717,6 +7717,11 @@ Recycler::DeleteGuestArena(ArenaAllocator * arenaAllocator)`
`7717`	`7717`	`{`
`7718`	`7718`	`guestArenaList.RemoveElement(&HeapAllocator::Instance, guestArenaAllocator);`
`7719`	`7719`	`}`
	`7720`	`+`
	`7721`	`+ // Any time a root is removed during a GC, it indicates that an exhaustive`
	`7722`	`+ // collection is likely going to have work to do so trigger an exhaustive`
	`7723`	`+ // candidate GC to indicate this fact`
	`7724`	`+ this->CollectNow<CollectExhaustiveCandidate>();`
`7720`	`7725`	`}`
`7721`	`7726`
`7722`	`7727`	`#ifdef LEAK_REPORT`
Original file line number	Diff line number	Diff line change
`@@ -1247,11 +1247,21 @@ class Recycler`
`1247`	`1247`	`void UnregisterExternalGuestArena(ArenaData* guestArena)`
`1248`	`1248`	`{`
`1249`	`1249`	`externalGuestArenaList.Remove(&NoThrowHeapAllocator::Instance, guestArena);`
	`1250`	`+`
	`1251`	`+ // Any time a root is removed during a GC, it indicates that an exhaustive`
	`1252`	`+ // collection is likely going to have work to do so trigger an exhaustive`
	`1253`	`+ // candidate GC to indicate this fact`
	`1254`	`+ this->CollectNow<CollectExhaustiveCandidate>();`
`1250`	`1255`	`}`
`1251`	`1256`
`1252`	`1257`	`void UnregisterExternalGuestArena(ArenaData** guestArena)`
`1253`	`1258`	`{`
`1254`	`1259`	`externalGuestArenaList.RemoveElement(&NoThrowHeapAllocator::Instance, guestArena);`
	`1260`	`+`
	`1261`	`+ // Any time a root is removed during a GC, it indicates that an exhaustive`
	`1262`	`+ // collection is likely going to have work to do so trigger an exhaustive`
	`1263`	`+ // candidate GC to indicate this fact`
	`1264`	`+ this->CollectNow<CollectExhaustiveCandidate>();`
`1255`	`1265`	`}`
`1256`	`1266`
`1257`	`1267`	`#ifdef RECYCLER_TEST_SUPPORT`