ALL the Browsers #41

kriskowal · 2014-05-26T04:51:45Z

This is continued work to reimplement ASAP with greater rigor, accounting for all browsers. This includes additional automation for verifying the tests pass in all browsers available on Saucelabs, automation we can extend to perform A/B tests, benchmarks, experiments, and coverage analysis.

These are snapshots of the Saucelabs test results matrix for browser windows, and then browser workers. Grey denotes that Workers themselves are unavailable and the tests were not run. The tests are those in test/asap-test.js. These were ported from mocha to a minimal test scaffold designed to integrated with both Node.js and Saucelabs just for this project.

Split implementations for browsers and Node.js.
Now using Browserify-style module redirects.
Split implementations of asap and rawAsap.
rawAsap is used by asap, is about twice as fast, but can be
stalled by exceptions.
Support callable tasks, as a generalization of functions.
Use S3 and Mr/s to bundle and publish tests online, for development,
continuous unit testing, and for ad hoc experiments.
See scripts/publish-bundle.js.
Use Saucelabs to create browser compatibility matrices
for development, continuous unit testing, and releases.
See scripts/saucelabs.js
Reevaluate the use of various implementations based on data
obtained experimentally about all relevant browsers, using
Saucelabs.
Add ASAP Gauntlet benchmark draft.
Reimplement tests without a test framework.
Use a back up plan for engines that tend to forget timers (Firefox 8–18)
Address problem that Internet Explorer 10 web workers dequeue timers in LIFO order instead of FIFO.

- Split implementations for browsers and Node.js. Now using Browserify-style module redirects. - Split implementations of asap and rawAsap. rawAsap is used by asap, is about twice as fast, but can be stalled by exceptions. - Support callable tasks, as a generalization of functions. - Use S3 and Mr/s to bundle and publish tests online, for development, continuous unit testing, and for ad hoc experiments. See `scripts/publish-bundle.js`. - Use Saucelabs to create browser compatibility matrices for development, continuous unit testing, and releases. See `scripts/saucelabs.js` - Reevaluate the use of various implementations based on data obtained experimentally about all relevant browsers, using Saucelabs. - Add ASAP Gauntlet benchmark draft. - Reimplement tests without a test framework.

Fix issue with postponing errors in Internet Explorer workers, since setTimeout is not FIFO in that context. Add Saucelabs automation for workers. Add encrypted credentials for TravisCI and script for producing them. Use JSHint for linting.

To reflect new development mode dependencies

Addressed problems that arose from running asap in web workers in Firefox 8-18. This included increasing the timeout for the test suite in the case of workers, and a back-up plan if timers are ignored.

kriskowal · 2014-05-26T05:00:43Z

Unfortunately, the TravisCI integration requires additional effort.

stefanpenner · 2014-05-26T05:02:52Z

browser-raw.js

+        requestFlush();
+        flushing = true;
+    }
+    // Not every browser supports `push` on arrays.


which don't?

Nothing we actually care about. It’s been with us since Firefox 1.0, but before IE 5.5, Netscape Navigator at some point. We’re talking about ancient history. In any case, it’s one less function call. Would be interesting to see A/B benchmark numbers.

Comment is misleading, even if it's a good perf optimization (which I have heard is true).

With narrative explaining why. Fixes #21

juandopazo · 2014-05-28T15:47:36Z

wow

    such asap

domenic · 2014-05-28T20:59:59Z

.npmignore

@@ -0,0 +1,6 @@
+**


We could use the files property in package.json.

Addressed in working copy…

kriskowal · 2014-05-28T23:14:39Z

Addressed comments so far. Back to the pool for feedback from @domenic and @rkatic.

rkatic · 2014-05-29T01:41:42Z

Will try to take a look soon.
On May 28, 2014 7:14 PM, "Kris Kowal" notifications@github.com wrote:

Addressed comments so far. Back to the pool for feedback from @domenichttps://github.com/domenicand
@rkatic https://github.com/rkatic.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/41#issuecomment-44476655
.

The full suite exceeded the 50 minute limit.

kriskowal · 2014-05-29T17:12:13Z

So, TravisCI works now. However, I need to publish a fix to Q-IO for the Saucelabs test matrices to get copied around properly. That is not a blocker, but to be aware.

To allow the Saucelabs test matrix to be copied to S3 properly.

domenic · 2014-06-01T05:15:37Z

README.md

+function Task() {}
+
+Task.new = function () {
+    if (this.freeTasks.length) {


Is it this.freeTasks, or Task.freeTasks?

this.freeTasks when this === Task.

rkatic · 2014-06-01T05:30:09Z

I took a look to the node and the browser implementations of ASAP (have no time to also review tests and the testing infrastructure right now), and I don't have any major objection. For me, this is good to go.

I just have a minor objection, that could be probably handled later.
The domain module is required even if the user of ASAP never uses it.
Even if it's the simplest approach, it's probably not the best behavior we can adopt.
Loading the domain module, an additional complexity is added to the system, "wrapping" many built-in functions (setTimeout, nextTick, setImmediate, setIntervall...) and "unlocking" domain related logic in different built-in modules. We should probably avoid this if not necessary, requiring the domain module only if domains are actually in use.

So in raw.js, we can

var parentDomain = process.domain;
if (parentDomain) {
   if (!domain) domain = require("domain");
   domain.active = process.domain = null;
}

and in asap.js, we can replace domain.active with process.domain.

In this way we can remove require("domain") on top of both files.

domenic · 2014-06-01T05:33:55Z

browser-raw.js

+// a task throws an exception.
+var index = 0;
+// If a task schedules additional tasks recursively, the task queue can grown
+// unbounded. To prevent memory excaustion, the task queue will periodically


Typo "exhaustion"

domenic · 2014-06-01T05:39:14Z

+1 to @rkatic's comments.

Also: it seems like a lot of generated files are being checked in. Perhaps they could not be? Unsure how that would work though.

rkatic · 2014-06-01T07:17:37Z

raw.js

+        // shift tasks off the queue after they have been executed.
+        // Instead, we periodically shift 1024 tasks off the queue.
+        if (index > capacity) {
+            queue.splice(0, capacity);


Here splice also unnecessary generates an array of 1024 tasks.
How about queue = queue.slice(capacity);?

Both strategies generate a garbage array. Only way around this is to manually shift and truncate.

- Note that raw asap does not handle domains and how to do so manually. - Clarify message channels, what they would do, why we don't use them. - Use try/finally instead of try/catch/finally to avoid munging stack traces. - Unroll queue shifts and truncate to avoid generating garbage.

kriskowal · 2014-06-08T05:04:32Z

Addressed comments by @rkatic and @domenic so far. If TravisCI passes, I will merge and publish unless there are any objections.

rkatic · 2014-06-08T05:52:14Z

browser-asap.js

+            // by name.
+            setImmediate(function () {
+                throw error;
+            });


Can we be more clear why kriskowal/q#396 does not apply here?

@kriskowal in asap.js, you missed to remove domain loading and to replace
domain.active with process.domain.
On Jun 8, 2014 11:37 AM, "Kris Kowal" notifications@github.com wrote:

In browser-asap.js:

// This hook exists purely for testing purposes.

// Its name will be periodically randomized to break any code that

// depends on its existence.

asap.onerror(error);

} else if (hasSetImmediate) {

// In WebWorkers on Internet Explorer 10 and 11, the setTimeout

// function is not FIFO.

// Thankfully these browsers have setImmediate, which behaves

// correctly.

// In all other known cases, setTimeout is FIFO, including non

// Worker contexts in the exact same browsers.

// Note that in Internet Explorer 10, setImmediate must be called

// by name.

setImmediate(function () {

throw error;

});

✓

—
Reply to this email directly or view it on GitHub
https://github.com/kriskowal/asap/pull/41/files#r13524709.

rkatic · 2014-06-08T05:55:42Z

@kriskowal skipped #41 (comment)?

kriskowal · 2014-06-08T05:59:10Z

@rkatic Thanks. I missed that one.

- Lazy load domains module - Elaborate on why I elected setImmediate for error rethrow over setTimeout.

@rkatic

Per @rkatic. Thanks.

kriskowal · 2014-06-08T18:50:44Z

I’ve merged this, mostly to get it on the main line. For further issues, please open tickets.

kriskowal added 5 commits May 24, 2014 21:20

Workers

4054855

Fix issue with postponing errors in Internet Explorer workers, since setTimeout is not FIFO in that context. Add Saucelabs automation for workers. Add encrypted credentials for TravisCI and script for producing them. Use JSHint for linting.

Update package.json

d326e37

To reflect new development mode dependencies

Fix bug in annotations affecting continuous integration

6f6aebe

Run in web workers on all browsers

57708dd

Addressed problems that arose from running asap in web workers in Firefox 8-18. This included increasing the timeout for the test suite in the case of workers, and a back-up plan if timers are ignored.

kriskowal changed the title ~~Revised~~ Test ALL the Browsers May 26, 2014

kriskowal changed the title ~~Test ALL the Browsers~~ ALL the Browsers May 26, 2014

stefanpenner reviewed May 26, 2014
View reviewed changes

kriskowal added 2 commits May 26, 2014 08:17

Follow-up: tune JSHint configuration, etc

cc90f85

Dedup weak map for TravisCI

935263b

Eliminate use of setImmediate

2a2a1b4

With narrative explaining why. Fixes #21

kriskowal assigned domenic May 28, 2014

domenic mentioned this pull request May 28, 2014

Testing and Continuous Build? domenic/html-as-custom-elements#11

Open

Improve narrative comments

ebeb472

domenic reviewed May 28, 2014
View reviewed changes

kriskowal self-assigned this May 28, 2014

kriskowal added 2 commits May 28, 2014 16:12

Took a hint

029667b

Addressed feedback for #41

9e8233a

kriskowal assigned domenic and unassigned kriskowal May 28, 2014

kriskowal added 2 commits May 28, 2014 19:56

Isolate issues with TravisCI and WeakMap

12c49a7

Scale back to spot tests on TravisCI

6dbaed0

The full suite exceeded the 50 minute limit.

Uprev Q-IO

47c5341

To allow the Saucelabs test matrix to be copied to S3 properly.

domenic reviewed Jun 1, 2014
View reviewed changes

rkatic reviewed Jun 1, 2014
View reviewed changes

rkatic reviewed Jun 8, 2014
View reviewed changes

kriskowal added 2 commits June 8, 2014 08:35

Incorporate additional feedback from Robert Katić

e810305

- Lazy load domains module - Elaborate on why I elected setImmediate for error rethrow over setTimeout.

Remove dependency on domain in asap

917bcb6

Per @rkatic. Thanks.

kriskowal merged commit 917bcb6 into master Jun 8, 2014

kriskowal deleted the revised branch June 8, 2014 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ALL the Browsers #41

ALL the Browsers #41

kriskowal commented May 26, 2014

kriskowal commented May 26, 2014

stefanpenner May 26, 2014

kriskowal May 26, 2014

domenic May 28, 2014

kriskowal Jun 8, 2014

juandopazo commented May 28, 2014

domenic May 28, 2014

kriskowal May 28, 2014

kriskowal commented May 28, 2014

rkatic commented May 29, 2014

kriskowal commented May 29, 2014

domenic Jun 1, 2014

kriskowal Jun 7, 2014

rkatic commented Jun 1, 2014

domenic Jun 1, 2014

kriskowal Jun 8, 2014

domenic commented Jun 1, 2014

rkatic Jun 1, 2014

kriskowal Jun 8, 2014

kriskowal Jun 8, 2014

kriskowal commented Jun 8, 2014

rkatic Jun 8, 2014

kriskowal Jun 8, 2014

rkatic Jun 8, 2014

kriskowal Jun 8, 2014

rkatic commented Jun 8, 2014

kriskowal commented Jun 8, 2014

kriskowal commented Jun 8, 2014

ALL the Browsers #41

ALL the Browsers #41

Conversation

kriskowal commented May 26, 2014

kriskowal commented May 26, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juandopazo commented May 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriskowal commented May 28, 2014

rkatic commented May 29, 2014

kriskowal commented May 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkatic commented Jun 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Jun 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriskowal commented Jun 8, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkatic commented Jun 8, 2014

kriskowal commented Jun 8, 2014

kriskowal commented Jun 8, 2014