652221 - War on Spinning the Event Loop: use AsyncResource

Status: RESOLVED WONTFIX

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

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.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Part 0: minor test tweaks.

Comment 2

[Richard Newman [:rnewman]]

Attachment: Pointer to pull request

Comment 3

[Richard Newman [:rnewman]]

Sorry for the bugspam, folks; bzpatch's argument order is wrong in the helpstring.

Comment 4

[Richard Newman [:rnewman]]

Attachment: Pull request: parts 0 through 4.

Comment 5

[Richard Newman [:rnewman]]

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.

Comment 6

[Richard Newman [:rnewman]]

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.

Comment 7

[Richard Newman [:rnewman]]

Attachment: Parts 0 through 4, v2

Comment 8

[Richard Newman [:rnewman]]

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

[Philipp von Weitershausen [:philikon]]

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

Comment 10

[Richard Newman [:rnewman]]

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

Comment 11

[Richard Newman [:rnewman]]

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.

Comment 12

[Richard Newman [:rnewman]]

Attachment: Parts 5 through 11, v1

I don't expect all of these to get through without changes, but best to start somewhere!

All 101 tests pass with these changes.

Comment 13

[Philipp von Weitershausen [:philikon]]

We've abandoned this work in favour of a better approach (repositories, synchronizer, etc.)

Comment 14

[Philipp von Weitershausen [:philikon]]

Comment on attachment 538809 [details]
Parts 5 through 11, v1

Clearing review flag on WONTFIXed bug.