Bug 158999 – Options::useImmortalObjects is not safe for conservative GC

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Options::useImmortalObjects is not safe for conservative GC

[Geoffrey Garen]

Created attachment 281784 [details]
Patch

[Michael Saboff]

Comment on attachment 281784 [details]
Patch

r=me

[Joseph Pecoraro]

Comment on attachment 281784 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=281784&action=review

> Source/JavaScriptCore/heap/Heap.cpp:-1476
> -    if (Options::useImmortalObjects())
> -        markDeadObjects();

This is the only use of Options::useImmortalObjects. Should we remove the Option, or should we leave in legacy options even if they do nothing?

[Joseph Pecoraro]

Comment on attachment 281784 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=281784&action=review

> Source/JavaScriptCore/heap/MarkedAllocator.cpp:74
> +        bool isImmortal = block->state() != MarkedBlock::New && Options::useImmortalObjects();

I see it is now used here! Nevermind =)

[Geoffrey Garen]

Created attachment 281786 [details]
Patch

[Michael Saboff]

Comment on attachment 281786 [details]
Patch

LGTM

[Mark Lam]

Comment on attachment 281786 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=281786&action=review

> Source/JavaScriptCore/heap/MarkedAllocator.cpp:225
> +    if (Options::useImmortalObjects()) {

Make this UNLIKELY(Options::useImmortalObjects())?

[Geoffrey Garen]

Created attachment 281788 [details]
Patch for landing

[Geoffrey Garen]

Created attachment 281789 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 281789 [details]
Patch for landing

Clearing flags on attachment: 281789

Committed r202299: <http://trac.webkit.org/changeset/202299>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.