[webkit-reviews] review granted: [Bug 195698] Remove unneeded --tradeDestructorBlocks option. : [Attachment 364572] proposed patch.

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 195698: Remove unneeded --tradeDestructorBlocks option.
https://bugs.webkit.org/show_bug.cgi?id=195698

Attachment 364572: proposed patch.

https://bugs.webkit.org/attachment.cgi?id=364572&action=review




--- Comment #2 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 364572
  --> https://bugs.webkit.org/attachment.cgi?id=364572
proposed patch.

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

r=me

> Source/JavaScriptCore/ChangeLog:16
> +	   (!Options::tradeDestructorBlocks() && needsDestruction()).  This
assertion is
> +	   outdated because the BlockDirectory's m_empty set used to mean the
set of all
> +	   blocks that have no live (as in not reachable by GC) objects and
dead objects
> +	   also do not require destructors to be called on them.  The current
meaning of
> +	   m_empty is that it is the set of all blocks that have no live
objects,

OK, now, it is separated as "m_destructible".

> Source/JavaScriptCore/heap/LocalAllocator.cpp:190
>	       

OK, it is always calling `block->sweep(nullptr)`. So we can get blocks even if
m_directory is destructible (trade-destructor-blocks).