Summary: | The mutator should be able to perform increments of GC work | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, commit-queue, dbates, keith_miller, mark.lam, msaboff, saam | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Filip Pizlo
2017-01-27 14:31:28 PST
Created attachment 299962 [details]
work in progress
Created attachment 300158 [details]
possibly better version
Created attachment 300235 [details]
the patch
Comment on attachment 300235 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=300235&action=review r=me. > Source/JavaScriptCore/heap/SlotVisitor.cpp:552 > + if (status == IterationStatus::Continue) > + break; Using IterationStatus::Continue to signal we have no more work to do is confusing but I guess that's the pattern. > Source/WTF/wtf/DataLog.cpp:131 > + s_file = new (s_lockedFileData) LockedPrintStream(std::unique_ptr<FilePrintStream>(file)); oh wow... (In reply to comment #4) > Comment on attachment 300235 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=300235&action=review > > r=me. > > > Source/JavaScriptCore/heap/SlotVisitor.cpp:552 > > + if (status == IterationStatus::Continue) > > + break; > > Using IterationStatus::Continue to signal we have no more work to do is > confusing but I guess that's the pattern. It's unfortunate. I will we had generators. It's actually so confusing here: we say "Continue" to tell the forEachBlah to go to the next mark stack. We do that when the mark stack we were at had no work. So, if forEachBlah returns Continue then no mark stack had work. Hence, if it says Continue, we break. > > > Source/WTF/wtf/DataLog.cpp:131 > > + s_file = new (s_lockedFileData) LockedPrintStream(std::unique_ptr<FilePrintStream>(file)); > > oh wow... Comment on attachment 300235 [details] the patch r=me > Source/JavaScriptCore/ChangeLog:24 > + precise at keeping case as the new work-based incremental mode. keeping pace > Source/JavaScriptCore/ChangeLog:33 > + each other is a huge splay-latency speed-up on my iPad. It's also a CDjs progression. It does > + progress splay-throughput, but I think that's fine (the regression is 11%, the progression is did you mean *regress* splay-throughput? (In reply to comment #6) > Comment on attachment 300235 [details] > the patch > > r=me > > > Source/JavaScriptCore/ChangeLog:24 > > + precise at keeping case as the new work-based incremental mode. > > keeping pace Fixed. > > > Source/JavaScriptCore/ChangeLog:33 > > + each other is a huge splay-latency speed-up on my iPad. It's also a CDjs progression. It does > > + progress splay-throughput, but I think that's fine (the regression is 11%, the progression is > > did you mean *regress* splay-throughput? Yes. Fixed. Landed in https://trac.webkit.org/changeset/211448 |