Summary: | JSC::Heap should expose a richer API for requesting GCs | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, ysuzuki | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 171689 | ||||||
Attachments: |
|
Description
Filip Pizlo
2017-05-04 14:11:23 PDT
Created attachment 309101 [details]
the patch
Comment on attachment 309101 [details]
the patch
r=me
Comment on attachment 309101 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=309101&action=review > Source/JavaScriptCore/heap/Heap.cpp:984 > + } } style nit: I think there should be a newline between braces here. > Source/JavaScriptCore/heap/Heap.cpp:997 > + if (request.subsumedBy(request)) { This confuses me. Why are we calling subsumedBy always passing in the self object? > Source/JavaScriptCore/heap/Synchronousness.h:30 > +enum Synchronousness { style nit: How about an enum class here since this only used in a handful of locations Sorry did not mean to clear Geoff's r+ Comment on attachment 309101 [details]
the patch
r=ggaren+saam
(In reply to Saam Barati from comment #3) > Comment on attachment 309101 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309101&action=review > > > Source/JavaScriptCore/heap/Heap.cpp:984 > > + } } > > style nit: I think there should be a newline between braces here. > > > Source/JavaScriptCore/heap/Heap.cpp:997 > > + if (request.subsumedBy(request)) { > > This confuses me. Why are we calling subsumedBy always passing in the self > object? oops. That's a bug! I'll test the fix. > > > Source/JavaScriptCore/heap/Synchronousness.h:30 > > +enum Synchronousness { > > style nit: How about an enum class here since this only used in a handful of > locations |