Closed
Bug 652221
Opened 14 years ago
Closed 13 years ago
War on Spinning the Event Loop: use AsyncResource
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: rnewman, Assigned: rnewman)
References
Details
Attachments
(2 files, 3 obsolete files)
The first of the work item bugs for Bug 600059.
Let's start by eliminating our use of Resource non-invasively, moving functionality into *Cb versions of functions until we can bootstrap async higher and higher up the stack.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #535510 -
Attachment description: Pointer to pull request → Part 0: minor test tweaks.
Assignee | ||
Updated•13 years ago
|
Attachment #535510 -
Attachment is obsolete: true
Assignee | ||
Comment 2•13 years ago
|
||
Assignee | ||
Comment 3•13 years ago
|
||
Sorry for the bugspam, folks; bzpatch's argument order is wrong in the helpstring.
Updated•13 years ago
|
Summary: War on Sync: use AsyncResource → War on Spinning the Event Loop: use AsyncResource
Assignee | ||
Comment 4•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #535511 -
Attachment is obsolete: true
Assignee | ||
Comment 5•13 years ago
|
||
Comment on attachment 535514 [details]
Pull request: parts 0 through 4.
Part 0: Convert test_corrupt_keys.js to add_test/run_next_test.
Part 1: Add a test for URI construction and query modification.
Part 2: Add Utils.slices.
Part 3: Move async utilities into async.js. Add Async.spinningly as a temporary shim for wrapping Cb versions of functions.
Part 4: Move Resource.serverTime to AsyncResource, fix comments for AsyncResource.
Attachment #535514 -
Attachment description: Pointer to pull request → Pull request: parts 0 through 4.
Attachment #535514 -
Flags: review?(philipp)
Assignee | ||
Comment 6•13 years ago
|
||
Addressed comments:
* Re-split commits to address some cross-mojination
* Added URI tests to test_resource_async.js
* Renamed Async.spinningly to Async.makeSpinningCallback
* Misc comment improvements
* Make Utils.slices and Async.barrieredCallbacks into generators.
Working on one test hang before each individual commit passes all tests, as well as the whole set of commits, but all of the reviewed code so far is good to go.
Philipp, I will submit a fresh pull request for the parts you've reviewed and I've amended.
Assignee | ||
Comment 7•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #535618 -
Attachment description: Pointer to pull request → Parts 0 through 4, v2
Attachment #535618 -
Flags: review?(philipp)
Assignee | ||
Updated•13 years ago
|
Attachment #535514 -
Attachment is obsolete: true
Attachment #535514 -
Flags: review?(philipp)
Updated•13 years ago
|
Attachment #535618 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 8•13 years ago
|
||
Batch one:
http://hg.mozilla.org/services/services-central/rev/e89ff3fdbec0
http://hg.mozilla.org/services/services-central/rev/87291fa946cb
http://hg.mozilla.org/services/services-central/rev/533acc569efc
http://hg.mozilla.org/services/services-central/rev/3cc54a498f86
http://hg.mozilla.org/services/services-central/rev/eb0ab9014998
http://hg.mozilla.org/services/services-central/rev/1f3102c47d46
http://hg.mozilla.org/services/services-central/rev/124aef7f7e0b
http://hg.mozilla.org/services/services-central/rev/bac5be017ca7
Comment 9•13 years ago
|
||
(In reply to comment #8)
> Batch one:
Thanks. A merge commit mentioning the bug number would've been good (that's the only one that would've needed the r=philikon bit, too.
Assignee | ||
Comment 10•13 years ago
|
||
(In reply to comment #9)
> Thanks. A merge commit mentioning the bug number would've been good (that's
> the only one that would've needed the r=philikon bit, too.
Yeah, I considered that, but thought that this way would make for easier rebasing of part two, and a more granular history besides. I plan on using a merge commit for the inextricably linked final parts, but these stand alone.
My bad for omitting the bug number! Was kinda distracted by hg-git woes.
Assignee | ||
Comment 11•13 years ago
|
||
Gah, realized the reason why I'm still waiting for review for the rest is because I never formally asked for it.
Will fire up another push request.
Assignee: rnewman → nobody
Assignee | ||
Comment 12•13 years ago
|
||
I don't expect all of these to get through without changes, but best to start somewhere!
All 101 tests pass with these changes.
Assignee: nobody → rnewman
Attachment #538809 -
Flags: review?(philipp)
Comment 13•13 years ago
|
||
We've abandoned this work in favour of a better approach (repositories, synchronizer, etc.)
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Comment 14•13 years ago
|
||
Comment on attachment 538809 [details]
Parts 5 through 11, v1
Clearing review flag on WONTFIXed bug.
Attachment #538809 -
Flags: review?(philipp)
Updated•6 years ago
|
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•