Bug 192647 – The JSC shell should listen for memory pressure events and respond to them

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

...

[Saam Barati]

Created attachment 357197 [details]
patch

[EWS Watchlist]

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.

[Saam Barati]

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

[Keith Miller]

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.

[Saam Barati]

(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.

[Saam Barati]

Created attachment 357281 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 357281 [details]
patch for landing

Clearing flags on attachment: 357281

Committed r239195: <https://trac.webkit.org/changeset/239195>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46719817>

[Truitt Savell]

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

[Saam Barati]

(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?

[Truitt Savell]

It looks like it was a bot related issue effecting both the bots on that queue. Sorry for the noise.