Oilpan: Garbage collection during mixin construction |
|
Issue descriptionMixin construction currently forbids doing garbage collection. This makes it tricky to (a) perform arbitrary incremental or even concurrent marking during construction of such objects and (b) creates a separate path for object construction. This is a tracking bug for uniform object allocation that allows GC and/or marking during mixin construction. Design doc: https://docs.google.com/document/d/14U3JyYmJ6OzKEQhwO8H_BtPJXyHfx2v8uMQjr2OsNHQ
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9efdb6fb26376a826151d722add7689b0af917d commit c9efdb6fb26376a826151d722add7689b0af917d Author: Hans Wennborg <hans@chromium.org> Date: Wed Dec 05 11:55:57 2018 Revert "gc-plugin: Allow typdef as mixin marker in using macro" This reverts commit 84d989af7b54497c435347b4e9d6c84114c8b5e2. Reason for revert: The test doesn't pass: https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTLinux%2F4559%2F%2B%2Frecipes%2Fsteps%2Fgclient_runhooks%2F0%2Fstdout Original change's description: > gc-plugin: Allow typdef as mixin marker in using macro > > This allows us to get rid of the empty class marker when we decide to > check construction in a different way. The typedef does not require > memory in contrast to an empty class. > > Bug: 911662 > Change-Id: I99ab7e35e2b60dd0065f0db2483c419ae7b81002 > Reviewed-on: https://chromium-review.googlesource.com/c/1361712 > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Commit-Queue: Michael Lippautz <mlippautz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613915} TBR=haraken@chromium.org,mlippautz@chromium.org Change-Id: Ia777738ea594b1c83cc6d1ab73a315ef0e6812a8 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 911662, 912021 Reviewed-on: https://chromium-review.googlesource.com/c/1362901 Reviewed-by: Hans Wennborg <hans@chromium.org> Commit-Queue: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#613942} [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/BadPatternFinder.cpp [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/tests/heap/stubs.h [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/tests/missing_mixin_marker.cpp [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/tests/missing_mixin_marker.txt
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0393f0ea68d77ac037b20c6040fc19af915b34cc commit 0393f0ea68d77ac037b20c6040fc19af915b34cc Author: Michael Lippautz <mlippautz@chromium.org> Date: Fri Dec 07 14:29:39 2018 Reland "gc-plugin: Allow typdef as mixin marker in using macro" This reverts commit c9efdb6fb26376a826151d722add7689b0af917d. Bug: 911662 Change-Id: I78c93aceaf32850fcfecb2a714f3d9e6e0997b50 Reviewed-on: https://chromium-review.googlesource.com/c/1367728 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/master@{#614693} [modify] https://crrev.com/0393f0ea68d77ac037b20c6040fc19af915b34cc/tools/clang/blink_gc_plugin/BadPatternFinder.cpp [modify] https://crrev.com/0393f0ea68d77ac037b20c6040fc19af915b34cc/tools/clang/blink_gc_plugin/tests/heap/stubs.h [modify] https://crrev.com/0393f0ea68d77ac037b20c6040fc19af915b34cc/tools/clang/blink_gc_plugin/tests/missing_mixin_marker.cpp [modify] https://crrev.com/0393f0ea68d77ac037b20c6040fc19af915b34cc/tools/clang/blink_gc_plugin/tests/missing_mixin_marker.txt
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f1cd99ffe67f4d5baa29138671853ab2c4baad29 commit f1cd99ffe67f4d5baa29138671853ab2c4baad29 Author: Michael Lippautz <mlippautz@chromium.org> Date: Thu Dec 13 13:00:45 2018 heap: Conservatively scan objects in construction Switch to conservative scan for objects that have vtables and are currently under construction. The previous mechanism relied on (a) checking for an initialized vtable pointer and (b) partial dispatch to already initialized member fields through using Trace. The approach could not handle mixins as the virtual trace method could not dispatch to the object inheriting from the currently constructed one. We switch to conservatively scanning objects with vtables that are under construction. The benefit is not having to rely on vtable setup and that it also works during mixin construction. The drawback is being conservative which is currently not a blocker as stack-scan is also conservative. Bug: 911662 Change-Id: I08b2434ca1fb4015553899012aca7594332f87e1 Reviewed-on: https://chromium-review.googlesource.com/c/1361713 Commit-Queue: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#616282} [modify] https://crrev.com/f1cd99ffe67f4d5baa29138671853ab2c4baad29/third_party/blink/renderer/platform/heap/marking_visitor.cc [modify] https://crrev.com/f1cd99ffe67f4d5baa29138671853ab2c4baad29/third_party/blink/renderer/platform/heap/marking_visitor.h
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b47ddca7ff0bac44cee64cd60e3d9734b408b60a commit b47ddca7ff0bac44cee64cd60e3d9734b408b60a Author: Michael Lippautz <mlippautz@chromium.org> Date: Thu Jan 10 18:09:43 2019 heap: Uniform object construction Switch to a uniform object construction that does not differentiate between mixins and regular garabge collected objects and as a consequence can interrupt both construction cases with garbage collections. Object construction now uniformely works as follows: 1. The object under construction is marked so in the HeapObjectHeader. 2. Upon discovering such an object it is delayed in a separate marking worklist. 3. Upon hitting the main atomic pause all such objects are conservatively scanned for pointers without using the Trace method. Special cases: a. Mixins in construction: The HeapObjectHeader cannot retrived for such objects. A default GetTraceDescriptor() implementation returning a sentinel marker is used to discover that such objects should be delayed. b. Upon reaching a safepoint (e.g. no stack), in-construction objects are moved to a marking worklist that, similar to the regular marking worklist, allows for incremental processing. Effects: - No more TLS access for no-GC scope on mixin construction. - No more memory needed for the no-GC scope marker in mixin classes - MakeGarbageCollected should require less binary size as implementations can be aligned. - Incremental marking Start and Step operations are safe to be called with stack (even though it is preferable to not do so). - Object construction is safe to be used with a concurrent marker as it is ok for any objects (mixins as well as normal ones) to be published to the object graph during constructors. Change-Id: Ia39b5e76d551ed194612091167e2370b2d6a4e5f Bug: 911662 Reviewed-on: https://chromium-review.googlesource.com/c/1401070 Commit-Queue: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#621637} [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/core/dom/live_node_list_registry.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/core/dom/shadow_root.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/modules/webmidi/midi_dispatcher.h [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/garbage_collected.h [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/heap.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/heap.h [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/heap_test.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/heap_test_utilities.h [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/incremental_marking_test.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/marking_visitor.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/marking_visitor.h [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/persistent.h [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/thread_state.cc [modify] https://crrev.com/b47ddca7ff0bac44cee64cd60e3d9734b408b60a/third_party/blink/renderer/platform/heap/thread_state.h
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ad406608dd996a8a11ee2070114d77817f5a9084 commit ad406608dd996a8a11ee2070114d77817f5a9084 Author: Adithya Srinivasan <adithyas@chromium.org> Date: Thu Jan 10 21:54:16 2019 Revert "heap: Uniform object construction" This reverts commit b47ddca7ff0bac44cee64cd60e3d9734b408b60a. Reason for revert: Causes blink_heap_unittests to fail on Android CFI and Linux CFI builders (https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20CFI/12427) Original change's description: > heap: Uniform object construction > > Switch to a uniform object construction that does not differentiate between > mixins and regular garabge collected objects and as a consequence can interrupt > both construction cases with garbage collections. > > Object construction now uniformely works as follows: > 1. The object under construction is marked so in the HeapObjectHeader. > 2. Upon discovering such an object it is delayed in a separate marking worklist. > 3. Upon hitting the main atomic pause all such objects are conservatively > scanned for pointers without using the Trace method. > > Special cases: > a. Mixins in construction: The HeapObjectHeader cannot retrived for such > objects. A default GetTraceDescriptor() implementation returning a sentinel > marker is used to discover that such objects should be delayed. > b. Upon reaching a safepoint (e.g. no stack), in-construction objects are moved > to a marking worklist that, similar to the regular marking worklist, allows for > incremental processing. > > Effects: > - No more TLS access for no-GC scope on mixin construction. > - No more memory needed for the no-GC scope marker in mixin classes > - MakeGarbageCollected should require less binary size as implementations can be > aligned. > - Incremental marking Start and Step operations are safe to be called with stack > (even though it is preferable to not do so). > - Object construction is safe to be used with a concurrent marker as it is ok > for any objects (mixins as well as normal ones) to be published to the object > graph during constructors. > > Change-Id: Ia39b5e76d551ed194612091167e2370b2d6a4e5f > Bug: 911662 > Reviewed-on: https://chromium-review.googlesource.com/c/1401070 > Commit-Queue: Michael Lippautz <mlippautz@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Cr-Commit-Position: refs/heads/master@{#621637} TBR=ulan@chromium.org,haraken@chromium.org,keishi@chromium.org,mlippautz@chromium.org Change-Id: Ia573ff1511cbf57f079d6589d9427dd1bbfbdaa6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 911662 Reviewed-on: https://chromium-review.googlesource.com/c/1406085 Reviewed-by: Adithya Srinivasan <adithyas@chromium.org> Commit-Queue: Adithya Srinivasan <adithyas@chromium.org> Cr-Commit-Position: refs/heads/master@{#621759} [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/core/dom/live_node_list_registry.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/core/dom/shadow_root.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/modules/webmidi/midi_dispatcher.h [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/garbage_collected.h [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/heap.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/heap.h [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/heap_test.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/heap_test_utilities.h [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/incremental_marking_test.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/marking_visitor.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/marking_visitor.h [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/persistent.h [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/thread_state.cc [modify] https://crrev.com/ad406608dd996a8a11ee2070114d77817f5a9084/third_party/blink/renderer/platform/heap/thread_state.h
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/af99c7c05ecda52645509335a1f203cc3c097b18 commit af99c7c05ecda52645509335a1f203cc3c097b18 Author: Michael Lippautz <mlippautz@chromium.org> Date: Fri Jan 11 19:42:29 2019 Reland "heap: Uniform object construction"" Switch to a uniform object construction that does not differentiate between mixins and regular garabge collected objects and as a consequence can interrupt both construction cases with garbage collections. Object construction now uniformely works as follows: 1. The object under construction is marked so in the HeapObjectHeader. 2. Upon discovering such an object it is delayed in a separate marking worklist. 3. Upon hitting the main atomic pause all such objects are conservatively scanned for pointers without using the Trace method. Special cases: a. Mixins in construction: The HeapObjectHeader cannot retrived for such objects. A default GetTraceDescriptor() implementation returning a sentinel marker is used to discover that such objects should be delayed. b. Upon reaching a safepoint (e.g. no stack), in-construction objects are moved to a marking worklist that, similar to the regular marking worklist, allows for incremental processing. Effects: - No more TLS access for no-GC scope on mixin construction. - No more memory needed for the no-GC scope marker in mixin classes - MakeGarbageCollected should require less binary size as implementations can be aligned. - Incremental marking Start and Step operations are safe to be called with stack (even though it is preferable to not do so). - Object construction is safe to be used with a concurrent marker as it is ok for any objects (mixins as well as normal ones) to be published to the object graph during constructors. This reverts commit ad406608dd996a8a11ee2070114d77817f5a9084. Bug: 911662 Change-Id: I7babc6bac25b446a63783bf2c3d19e9b525d9e9a Reviewed-on: https://chromium-review.googlesource.com/c/1405991 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/master@{#622098} [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/core/dom/live_node_list_registry.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/core/dom/shadow_root.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/modules/webmidi/midi_dispatcher.h [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/garbage_collected.h [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/heap.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/heap.h [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/heap_test.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/heap_test_utilities.h [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/incremental_marking_test.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/marking_visitor.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/marking_visitor.h [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/persistent.h [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/thread_state.cc [modify] https://crrev.com/af99c7c05ecda52645509335a1f203cc3c097b18/third_party/blink/renderer/platform/heap/thread_state.h |
|
►
Sign in to add a comment |
|
Comment 1 by bugdroid1@chromium.org
, Dec 5