1421358 - Remove GC infrastructure for NotifyDidPaint

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P3)

Description

[Andrew McCreight [:mccr8]]

The browser no longer uses the JS::NotifyDidPaint() API that was added for IGC. I think Olli changed how it works. In bug 1421355, I'll remove the public API for this, but it seems like there's a bunch of JS engine side code related to it that could be removed.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Yeah, these days we try hard to use idle time, and if there isn't any idle time, there isn't really any good time to run collectors so we fallback to timers.
vsync is still prioritized, so refresh driver should run before timers.

Comment 2

[Adrian Wielgosik [:adrian17]]

Attachment: Bug 1421358 - Remove GCRuntime::notifyDidPaint and refresh_frame_slices.enabled pref.

Review commit: https://reviewboard.mozilla.org/r/207666/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207666/

Comment 3

[Jon Coppeard (:jonco)]

Comment on attachment 8936941 [details]
Bug 1421358 - Remove GCRuntime::notifyDidPaint and refresh_frame_slices.enabled pref.

https://reviewboard.mozilla.org/r/207666/#review213544

Looks great, thanks for doing this!

::: js/public/GCAPI.h:385
(Diff revision 1)
>      D(SET_DOC_SHELL)                            \
>      D(DOM_UTILS)                                \
>      D(DOM_IPC)                                  \
>      D(DOM_WORKER)                               \
>      D(INTER_SLICE_GC)                           \
> -    D(REFRESH_FRAME)                            \
> +    D(UNUSED3)                                  \

We don't seems to have UNUSED1 so you can use that.

Comment 4

[Adrian Wielgosik [:adrian17]]

Comment on attachment 8936941 [details]
Bug 1421358 - Remove GCRuntime::notifyDidPaint and refresh_frame_slices.enabled pref.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207666/diff/1-2/

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Pushed to try on behalf of Adrian:

remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ee9e01c19e6a1b16b5ece02bf6e56607867b653

If it's green, feel free to mark it as checkin-needed (I'll pay attention too just in case).

Thanks a lot for the patch!

Comment 6

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 11 changes to 11 files
remote: (040bfe750c97 modifies servo/components/script/script_runtime.rs from Servo; not enforcing peer review)
remote: (040bfe750c97 modifies servo/resources/prefs.json from Servo; not enforcing peer review)
remote: (1 changesets contain changes to protected servo/ directory: 040bfe750c97)
remote: ************************************************************************
remote: you do not have permissions to modify files under servo/
remote: 
remote: the servo/ directory is kept in sync with the canonical upstream
remote: repository at https://github.com/servo/servo
remote: 
remote: changes to servo/ are only allowed by the syncing tool and by sheriffs
remote: performing cross-repository "merges"
remote: 
remote: to make changes to servo/, submit a Pull Request against the servo/servo
remote: GitHub project
remote: ************************************************************************
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.e_prevent_vendored hook failed
abort: push failed on remote

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Ouch, yeah, servo/ files shouldn't be touched, I'll just strip those changes.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30237d58eb35
Remove GCRuntime::notifyDidPaint and refresh_frame_slices.enabled pref. r=jonco

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/30237d58eb35