Closed Bug 1373046 Opened 7 years ago Closed 7 years ago

Use idle dispatch for IncrementalFinalizeRunnable

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: billm, Assigned: bevis)

References

Details

Attachments

(1 file, 1 obsolete file)

This would be good to do and also would help with runnable labeling (since I think we don't need to bother labeling idle runnables for now).
Need to use idle + timeout for this too to ensure objects are released at some point
I don't really grok Olli's comment 1, but I think that could be deferred to a
later point anyway?
Attachment #8896383 - Flags: review?(wmccloskey)
This bug is or was about idle dispatch, and if idle dispatch is used, need to use idle + timeout to ensure objects are released soon enough in case there isn't any idle time.
Oh, oh, I see.  Instead of labeling the runnable, we idle dispatch it, and then its unlabeledness doesn't really matter.

What is a reasonable timeout value, then?  Do we have guidelines on choosing those, or is it just "whatever sounds good and the reviewer thinks is OK?"
Comment on attachment 8896383 [details] [diff] [review]
label IncrementalFinalizeRunnable dispatches

Review of attachment 8896383 [details] [diff] [review]:
-----------------------------------------------------------------

I think we should just use idle dispatch. It's not much more work. This code, which is for a similar purpose, uses a 2.5 second timeout:
http://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/js/xpconnect/src/XPCJSRuntime.cpp#143
Maybe we should just do that?
Attachment #8896383 - Flags: review?(wmccloskey)
Address the suggestion in comment 5.

(Try to share some loading. Hope you don't mind, Nathan)

Treeherder result looks fine:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=637c30a701d95a6484b521b9d5eceaf94d4b01e7
Assignee: nobody → btseng
Attachment #8896383 - Attachment is obsolete: true
Attachment #8897400 - Flags: review?(wmccloskey)
(In reply to Bevis Tseng [:bevis][:btseng] from comment #6)
> Address the suggestion in comment 5.
> 
> (Try to share some loading. Hope you don't mind, Nathan)

That's quite all right, thank you!
Attachment #8897400 - Flags: review?(wmccloskey) → review+
Pushed by btseng@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6668a2b3f65a
Use idle dispatch for IncrementalFinalizeRunnable. r=billm
https://hg.mozilla.org/mozilla-central/rev/6668a2b3f65a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: