Summary: | Make the collector's fixpoint smart about scheduling work | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 165911, 165963 | ||||||||||||
Bug Blocks: | 165909, 166828, 166827, 166831 | ||||||||||||
Attachments: |
|
Description
Filip Pizlo
2016-12-15 12:18:54 PST
I'm imagining that each of the phases of the weak fixpoint would be put into a SharedTask or some even smarter object, and then we'd be able to write some scheduler that runs them in an order that reduces the average number of executions. *** Bug 165963 has been marked as a duplicate of this bug. *** I was originally thinking that in bug 165963, I would implement the infrastructure that's needed to intelligently schedule work and then in this bug I would use that infrastructure to get an optimization. I ended up doing both of those things in bug 165963. Created attachment 298310 [details]
possible patch
Created attachment 298316 [details]
the patch
Attachment 298316 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/MarkingConstraintSet.cpp:201: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:37: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:38: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:39: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:49: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:50: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:51: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:52: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
Total errors found: 11 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 298320 [details]
the patch
Attachment 298320 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/MarkingConstraintSet.cpp:201: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:37: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:38: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:39: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:49: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:50: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:51: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:52: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
Total errors found: 11 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 298321 [details]
the patch
Attachment 298321 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/MarkingConstraintSet.cpp:201: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:37: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:37: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:38: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:39: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:49: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:50: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:51: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:52: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/heap/MarkingConstraint.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4]
Total errors found: 11 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 298321 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=298321&action=review r=me with some minor comments. > Source/JavaScriptCore/heap/MarkingConstraintSet.cpp:124 > +void MarkingConstraintSet::add( > + std::unique_ptr<MarkingConstraint> constraint) Nit, put this on one line. > Source/JavaScriptCore/heap/SpaceTimeMutatorScheduler.cpp:2 > + * Copyright (C) 2016 Apple Inc. All rights reserved. 2017 > Source/JavaScriptCore/heap/SpaceTimeMutatorScheduler.h:2 > + * Copyright (C) 2016 Apple Inc. All rights reserved. 2017 (In reply to comment #11) > Comment on attachment 298321 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=298321&action=review > > r=me with some minor comments. > > > Source/JavaScriptCore/heap/MarkingConstraintSet.cpp:124 > > +void MarkingConstraintSet::add( > > + std::unique_ptr<MarkingConstraint> constraint) > > Nit, put this on one line. Done. > > > Source/JavaScriptCore/heap/SpaceTimeMutatorScheduler.cpp:2 > > + * Copyright (C) 2016 Apple Inc. All rights reserved. > > 2017 You're commenting on the pseudofile that the patch is showing you. In fact, SpaceTimeMutatorScheduler.cpp:2 is: * Copyright (C) 2016-2017 Apple Inc. All rights reserved. > > > Source/JavaScriptCore/heap/SpaceTimeMutatorScheduler.h:2 > > + * Copyright (C) 2016 Apple Inc. All rights reserved. > > 2017 Ditto. Landed in https://trac.webkit.org/changeset/210521 Fixed cloop in https://trac.webkit.org/changeset/210530 |