[webkit-reviews] review granted: [Bug 204555] [JSC] Introduce IsoHeapCellType : [Attachment 384252] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Nov 24 09:49:59 PST 2019
Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 204555: [JSC] Introduce IsoHeapCellType
https://bugs.webkit.org/show_bug.cgi?id=204555
Attachment 384252: Patch
https://bugs.webkit.org/attachment.cgi?id=384252&action=review
--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 384252
--> https://bugs.webkit.org/attachment.cgi?id=384252
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=384252&action=review
Nice work. r=me with suggestions.
> Source/JavaScriptCore/heap/IsoHeapCellType.h:35
> +class IsoHeapCellType final : public HeapCellType {
> +public:
> + IsoHeapCellType()
> + : HeapCellType(CellAttributes(CellType::needsDestruction ?
NeedsDestruction : DoesNotNeedDestruction, HeapCell::JSCell))
If I understand you correctly, you intend to only use IsoHeapCellType for
destructible types, yes? Why not:
1. name it IsoHeapDestructibleCellType?
2. Always pass NeedsDestruction down to HeapCellType constructor here?
3. static_assert(CellType::needsDestruction)?
> Source/JavaScriptCore/heap/IsoHeapCellType.h:53
> + DestroyFunc()(vm, cell);
Why not just go direct to CellType::destroy(cell) here?
More information about the webkit-reviews
mailing list