Closed Bug 1421358 Opened 2 years ago Closed 2 years ago

Remove GC infrastructure for NotifyDidPaint

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: mccr8, Assigned: adrian17)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

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.
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.
Keywords: good-first-bug
Priority: -- → P3
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.
Attachment #8936941 - Flags: review?(jcoppeard) → review+
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!
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
Ouch, yeah, servo/ files shouldn't be touched, I'll just strip those changes.
Assignee: nobody → adrian.wielgosik
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30237d58eb35
Remove GCRuntime::notifyDidPaint and refresh_frame_slices.enabled pref. r=jonco
https://hg.mozilla.org/mozilla-central/rev/30237d58eb35
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.