Summary: | Unify Bitmap math loops in MarkedBlock::Handle::specializedSweep(). | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||||
Status: | REOPENED --- | ||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, commit-queue, ews-watchlist, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 215312 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Mark Lam
2020-06-18 11:40:23 PDT
Created attachment 402224 [details]
proposed patch.
Created attachment 402225 [details]
proposed patch.
Comment on attachment 402225 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=402225&action=review > Source/JavaScriptCore/ChangeLog:11 > + understand than then old code. /then/the/ > Source/JavaScriptCore/ChangeLog:38 > + 5. Aldo fixed some typos in comments. /Aldo/Also/ Comment on attachment 402225 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=402225&action=review r=me > Source/JavaScriptCore/heap/MarkedBlockInlines.h:345 > + if (emptyMode == NotEmpty) { This section is *much* more readable than before, thanks! Comment on attachment 402225 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=402225&action=review > Source/JavaScriptCore/heap/MarkedBlockInlines.h:369 > + // At this point, a set bit in freeAtoms represents live cells. this is no longer true, right? Opposite Comment on attachment 402225 [details]
proposed patch.
reinstating Robin's r+. I'm not done reviewing though
Comment on attachment 402225 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=402225&action=review > Source/JavaScriptCore/heap/MarkedBlockInlines.h:367 > + bool isEmpty = !anyBits; alternatively, you can just set this to false, then to true when iterating freeAtoms.forEachSetBit below Comment on attachment 402225 [details]
proposed patch.
r=me too
Comment on attachment 402225 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=402225&action=review >> Source/JavaScriptCore/heap/MarkedBlockInlines.h:367 >> + bool isEmpty = !anyBits; > > alternatively, you can just set this to false, then to true when iterating freeAtoms.forEachSetBit below Good point. Thanks. >> Source/JavaScriptCore/heap/MarkedBlockInlines.h:369 >> + // At this point, a set bit in freeAtoms represents live cells. > > this is no longer true, right? Opposite Will fix. Created attachment 402261 [details]
patch for landing.
Thanks for the reviews. Landed in r263252: <http://trac.webkit.org/r263252>. Re-opened since this is blocked by bug 215312 |