featureDetection: eval and fromJson change

[James Burke]

The featureDetection branch has this for the definition of eval:

dojo["eval"] = function(){
    if (arguments.length == 1 || !arguments[1]) { 
        return eval(arguments[0]);
    }

    // NOTE: Script injection here if post load

    return (new Function(arguments[0]))();
};

I do not see how the return (new Function()) section would ever be called? IIRC, we normally only pass one argument to eval, and doing a quick search under the dojo/ dir in the branch only shows one argument only being passed to eval.

[James Burke]

A related change, in fromJson:

dojo.fromJson = function(/*String*/ json){
    return (new Function('return (' + json + ')'))(); // Object
};

Bill said he did a quick test using this code:

       var start = new Date();
       for(var i=0; i<10000; i++)
               dojo.fromJson("{a: 1, b: 2}");
       console.log("elapsed: " + (new Date() - start));

and the Function approach is generally slower:

IE6:

old: 594 new: 598

IE7:

old: 594 new: 641

IE8:

old: 219 new: 172

FF3.5/win:

old: 85 new: 165

Safari4/mac:

old: 23 new: 308

FF3.5/mac:

old: 147 new: 3000

This site also found similar results: ​http://weblogs.asp.net/yuanjian/archive/2009/03/22/json-performance-comparison-of-eval-new-function-and-json.aspx

Are there other benefits to the new Function approach?

At least for the fromJson method, I would prefer to delegate to native JSON.parse if available then just use eval. We have backwards compatibility problem described in #xxx, but generally I favor switching to a new method, maybe dojo.jsonParse() that does the JSON.parse or eval branch based on a capability/feature detection check.

[Les]

See also this ticket: ​http://trac.dojotoolkit.org/ticket/7053

[Les]

Also, see this jQuery ticket: ​http://dev.jquery.com/ticket/4680

[bill]

Les, thanks for finding those links.

I noticed the links from the jquery page saying that firebug scewed the results... the above result for FF3.5/mac had firebug enabled (although the FF3.5/win one didn't), so I reran FF3.5/mac after disabled firebug:

FF3.5/mac:

old: 116 new: 233

The existing method using eval is still faster than using Function.

[Les]

Looks like the jQuery team picked a slower method. I also checked the latest Ext Core 3.0, and they use eval, not new Function. Both Ext and jQuery check if there's native JSON support, though.

[Mike Wilson]

There's some discussion in #744 that might be interesting, f ex about debuggability.

[Les]

At least for the fromJson method, I would prefer to delegate to native JSON.parse if available then just use eval.

I'd recommend adding a configuration option to allow suppressing delegation to native JSON methods even if they are available, i.e. eval would be used even when native JSON methods are available.

[James Burke]

As bug #8111 points out, there is likely issues with fromJson using native JSON.parse since some code depends on fromJson for some non-json things. So it is more likely we would use new method names and change code where appropriate to use the new methods.

[bill]

Closing as per comments above.