Skip to content

Instantly share code, notes, and snippets.

@lgsunnyvale
Forked from zhuzhuaicoding/feedback.md
Created November 20, 2012 22:15
Show Gist options
  • Save lgsunnyvale/4121620 to your computer and use it in GitHub Desktop.
Save lgsunnyvale/4121620 to your computer and use it in GitHub Desktop.
Feedback to "Async Javascript"

HI Trevor Burnham:

I'm a reader of your Async JavaScript book, I bought it on 23rd March and finished it on 26rd March, it is really a treasure to me, probably the best book about async programming in JavaScript I have ever read.

After finished this book, I found there are a few problems, as well as a set of my questions, exists, so I wrote this mail to you, expecting you could give a brief look.

Since I'm not a native English speaker, my English is really poor, so if there is any ambiguousness, please feedback to me so I could have a change to state it more clearly.

Fist of all, I would like to list 9 problems I found in this book, most of which are produced by the difference between our perspectives, so feel free is you don't think they are problems.

  1. In chapter "The Javascript Event Model" - "Async primitives" you wrote:

    Only two async functions are part of the Javascript language itself: setTimeout and setInterval.

    But no, setTimeout and setInterval are not members of Javascript itself, ECMA262 nevers defines any async functions.

    In fact these 2 functions are defined by Timers in HTML specification and was adopted by most javascript host (such as NodeJS and all browsers), but still there are some host which does not provide an implement to setTimeout and setInterval, Microsoft's JScript for Windows is probably the most well knonw one, and also JSDB omits them (instead it provides an system.sleep function).

    Again, in chapter "Multi-threading with Workers" - "Web Workers" - Restrictions on Web Workers" you wrote:

    All a Worker can see is its own global object, self, and everything bundled with it: standard language feature like setTimeout/setInterval, plus the browsers Ajax methods.

    The problem is setTimeout and setInterval is not a language feature.

  2. In chapter "The Javascript Event Model" - "Async output" you wrote:

    Even the ubiquitous console.log is async in some environments.

    And you took Webkit-based browsers as an example, but still no, Webkit's console.log is not async, try this code:

     var i = 1;
     console.log(i);
     var i = 2;
     console.log(2);
    

    This outputs 1 and 2 correctly, whereas this code would output a result which seems to be async:

     var i = [1];
     console.log(i);
     i.push(2);
     // logs [1, 2]
    

    The reason is that Webkit's console has an intricate binding mechanism between the output of console and the real object in memory, in a certain scope the output in console would be synced to the state of the object (only if it is an object, not a primitive) when this object changes.

    So I don't think console.log is a good example for async methods in javascript.

  3. In chapter "The Javascript Event Model" - "Advanced timing methods" you wrote:

    When we schedule an event with setInterval and a 0ms delay, it should run as often as possible, right?

    OK, I can't force other to believe this is wrong, since many browsers (even the most standard ones) do run it as often as possible, but setTimeout defined in HTML specification asks host to run any job with a minimum delay of 4ms (see step 5).

    So as you wrote About 200/sec in latter paragraph, this is approaximately the right (and standard) value, while the extremely accurate RPS (run per second) value should be 250.

  4. In the same chapter you wrote:

    On the face of it, it looks like each trip to the event queue is adding overhead of about 5ms in modern browsers.

    According to the explanation above, it is really not the overhead of trip to the event queue, it's the problem of setTimeout function itself, try setImmediate in IE or postMessage on modern browsers, you would probably get a less overhead, and both setImmediate and postMessage uses event queue too.

  5. In chapter "Flow Control with Async.js" - "Async collection methods" you wrote:

    If we read the files sequentially, we don't have to deal with this limitation.

    Where limitation refers to the maximum number of files that Node (or any application process) can try to read simultaneously.

    Consider the number of request (assume we are in a web server role) node received simultaneously, even though every request opens one file at a time, when a great number of requests come together, it is also possible to exceed the limitation forced by OS, so it is not absolutely correct to say don't have to eal with this limitation.

    Since it may leads reader to believe the limication would never be reached if I read files sequentially and hence encounter unexpcted error, I suggest this book to provide a better explanation on this issue such as it greatly reduces the possibility one could exceed the limitation.

  6. In chapter "Flow Control with Async.js" - "Chaining async functions", the 2nd code example (only the problematic part) is:

     var fileTasks = fs.readdirSync('recipes')
       .filter(function(filename) {
         return fs.statSync(filename).isFile()
       })
         .map(function(filename, i) {
           return function(callback) {
             fs.readFile(filename, 'utf8', callback);
           };
         });
    

    Notice the indent of the line .map(..., it has 4 spaces while previous .filter line has only 2 spaces.

    The serial version of this code (the 1st code example of chapter "Flow Control with Async.js") is;

     fs.readdirSync('recipes')
       .filter(function(filename) {
         return fs.statSync(filename).isFile();
       })
       .forEach(function(filename) {
         concatenation += fs.readFileSync(filename, 'utf8');
       });
    

    The .foreach line has a consistent 2 spaces indent, so why the .map ine has a 4 spaces indent? is it a code style when envolving async operations, or is it just a trivial mistake?

    The same problem also exists in the 1st and 3rd code example in chapter "Flow Control with Async.js" - "Parallelizing async function" meeting repectively the .forEach line and .map line.

  7. In chapter "Flow Control with Async.js" - "Dynamic async queueing" - "When it's all done", the last code example is:

     var async = require('async');
     var fs = require('fs');
    
     var concatenation = '';
     var filenames = fs.readdirSync('recipes')
       .filter(function(filename) {
         return fs.statSync(filename).isFile();
       });
    
     function worker(i, callback) {
       fs.readFile(filenames[i], 'utf8', function(err, result) {
         if (err) throw err;
         results[i] = result; // here
         callback();
       });
     }
     var concurrency = 10;
     var queue = async.queue(worker, concurrency);
     var results = []; // here
     for (var i = 0; i < filenames.length; i++) {
       queue.push(i);
     }
     queue.drain = function() {
       console.log(concatenation = results.join(''));
     };
    

    Notice the 2 commented lines, the variable results is first referenced in function worker (though worker is not executed). This code works as expected, I'm just wondering is it a good practice to define a variable after a function which uses it? As a contrast, the concatenation variable is defined on top before any function (drain) which references it, so why results is defined in such a bottom place?

  8. In chapter "Async Problems and Solutions" - "Problem: Retrying async tasks" - "Solution 1: Callback-style", in the 1st code example, you made an invocation to callback when everything goes well:

     callback(null, Array.prototype.slice.call(arguments, 1));
    

    This is correct at first place, the problem is: it breaks the consistency.

    When I use attempAndRetry, I was expecting it to be transparent, that is, I could write my code like asyncTask(callback); and switch to attempAndRetry(asyncTask, 10, callback); without any modification to both asyncTask and callback.

    However, in your implement of attempAndRetry, you "zip" the callback arguments to an array, which means before introduce the attempAndRetry function, my callback's signature may be callback(err, stdin, stdout) (take child_process.exec of Node as an example), but after refactor my code to resident with attempAndRetry, my callback should change its signature to callback(err, args) and extract the args argument in callback's function body.

    Since this inconsistency in method signature is not a good thing to transparent function transformation, I would rather suggest callback.apply(this, arguments) here.

  9. In chapter "Async Problems and Solutions" - "Problem: Retrying async tasks" - "Solution 2: PubSub-style", in the 1st code example, I saw the 5th line was written:

     function retry = function() {
    

    This is obviously a typo, it should be var retry = function() { or function retry (preferred the latter one).

Other than problems cited above, I still have a few questions, it is appreciated if you could give a brief answer:

  1. This book warned me that a limitation on files can be open simultaneously would be forced by OS.

    Up to now I only knows there is a limitation, but have no idea what is the limitation, I tried google it but gain no luck, so is there any research on it, or is there any source that I could learn from?

  2. Almost all parts of this book are citing JavaScript is a single-threaded language with event model, but is JavaScript really single-threaded?

    I'm one of the translator of ECMA262 5th edition so I have already read the whole specification many times, but I didn't see any words related to thread or single in this specification, and ECMA262 provides neither an event model nor any async functions itself.

    In this words I'm not opposing the JavaScript is single-threaded point, I'm just wondering is it possible that we make a clear boundry between the language and the runtime (or host), and thus render a more accurately correct theory to readers.

Thanks for your reading :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment