Bug 192647 - The JSC shell should listen for memory pressure events and respond to them
Summary: The JSC shell should listen for memory pressure events and respond to them
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-12 16:33 PST by Saam Barati
Modified: 2018-12-14 13:04 PST (History)
19 users (show)

See Also:


Attachments
patch (12.72 KB, patch)
2018-12-12 16:59 PST, Saam Barati
keith_miller: review+
Details | Formatted Diff | Diff
patch for landing (12.70 KB, patch)
2018-12-13 18:31 PST, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2018-12-12 16:33:19 PST
...
Comment 1 Saam Barati 2018-12-12 16:59:10 PST
Created attachment 357197 [details]
patch
Comment 2 EWS Watchlist 2018-12-12 17:01:04 PST
Attachment 357197 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/MemoryPressureHandler.h:98:  Extra space before )  [whitespace/parens] [2]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Saam Barati 2018-12-12 17:02:06 PST
Comment on attachment 357197 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357197&action=review

> Source/WTF/wtf/MemoryPressureHandler.h:98
> +    WTF_EXPORT_PRIVATE void setDispatchQueue(dispatch_queue_t );

will fix style here
Comment 4 Keith Miller 2018-12-12 17:11:24 PST
Comment on attachment 357197 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357197&action=review

r=me. Can we do this for API users too?

> Source/WTF/wtf/MemoryPressureHandler.cpp:310
> +    dispatch_retain(queue);
> +    if (m_dispatchQueue)
> +        dispatch_release(m_dispatchQueue);
> +    m_dispatchQueue = queue;

If there's an easy way to assert this isn't called after installation I think we should do so. Otherwise maybe put a comment.
Comment 5 Saam Barati 2018-12-12 17:16:37 PST
(In reply to Keith Miller from comment #4)
> Comment on attachment 357197 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=357197&action=review
> 
> r=me. Can we do this for API users too?
Yeah we should. I'll open a followup to do so.

> 
> > Source/WTF/wtf/MemoryPressureHandler.cpp:310
> > +    dispatch_retain(queue);
> > +    if (m_dispatchQueue)
> > +        dispatch_release(m_dispatchQueue);
> > +    m_dispatchQueue = queue;
> 
> If there's an easy way to assert this isn't called after installation I
> think we should do so. Otherwise maybe put a comment.
Comment 6 Saam Barati 2018-12-13 18:31:28 PST
Created attachment 357281 [details]
patch for landing
Comment 7 WebKit Commit Bot 2018-12-13 20:05:49 PST
Comment on attachment 357281 [details]
patch for landing

Clearing flags on attachment: 357281

Committed r239195: <https://trac.webkit.org/changeset/239195>
Comment 8 WebKit Commit Bot 2018-12-13 20:05:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-12-13 20:06:27 PST
<rdar://problem/46719817>
Comment 10 Truitt Savell 2018-12-14 09:01:23 PST
The revision from https://trac.webkit.org/changeset/239195/webkit

May have caused Sierra WK1 Debug testing to become very flakey. This queue appears to have begun flaking 10-40 tests a run with some tests being constant timeouts which started around this patch

https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK1%20(Tests)/builds/10714

Most of the effected tests seem to be timeouts.

A test that is constantly timing out: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ftext%2Femoji-num-glyphs.html

Run History:
https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK1%20(Tests)?numbuilds=50
Comment 11 Saam Barati 2018-12-14 09:26:09 PST
(In reply to Truitt Savell from comment #10)
> The revision from https://trac.webkit.org/changeset/239195/webkit
> 
> May have caused Sierra WK1 Debug testing to become very flakey. This queue
> appears to have begun flaking 10-40 tests a run with some tests being
> constant timeouts which started around this patch
> 
> https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK1%20(Tests)/
> builds/10714
> 
> Most of the effected tests seem to be timeouts.
> 
> A test that is constantly timing out:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=fast%2Ftext%2Femoji-num-glyphs.html
> 
> Run History:
> https://build.webkit.org/builders/
> Apple%20Sierra%20Debug%20WK1%20(Tests)?numbuilds=50

Not really sure how that could be. This should only have an effect on the JSC shell. Are there other changes in the range?
Comment 12 Truitt Savell 2018-12-14 13:04:30 PST
It looks like it was a bot related issue effecting both the bots on that queue. Sorry for the noise.