4 ms·

Very worthwhile and well-designed writeup! I do wish that he had expanded the last argument to show what/how he fixed the performance problem. Also, does anyone

Very worthwhile and well-designed writeup! I do wish that he had expanded the last argument to show what/how he fixed the performance problem. Also, does anyone know why eval is faster than just defining the function? I mean wtf, why doesn't v8 make this optimization?

daeken 14y ago
He's not just evalling code, but building a function without loops and lots of lookups. That's not something you can generalize.
mistercow 14y ago
>Also, does anyone know why eval is faster than just defining the function? I mean wtf, why doesn't v8 make this optimization? There are three optimizations I see in the eval example: 1. The object is created as a literal rather than by property assignment (see http://jsperf.com/object-literals-vs-object-properties http://jsperf.com/object-literals-vs-object-properties). 2. The for loop loop is unrolled. 3. The column names are cached in the function and do not have to be retrieved each time. The reason these can't be done by v8 on its own is that they make assumptions that v8 can't guarantee. For the first two, you have to guarantee that the columns array is always the same length. For the first and last, you have to guarantee that the columns will always have the same names. So even though you take a big hit on the initial eval of the function, you'll likely make up for it by having a better-optimized function.
- rb12345 14y ago
  I noticed a possible issue with the code generation, which may cause the issues with excessive heap allocation that were reported: var code = 'return {\n'; columns.forEach(function(column) { code += '"' + column.name + '":' + 'parser.readColumnValue(),\n'; }); code += '};\n'; var parseRow = new Function('columns', 'parser', code); Instead of performing so many appends to a single string, I would try using map() and then join() the resulting strings: var code = ['return {\n', columns.map(function(column) { return '"' + column.name + '":' + 'parser.readColumnValue(),\n'; }).join(''), '};\n'].join(''); var parseRow = new Function('columns', 'parser', code); Of course, this depends on how often the parseRow function is created...
  - mistercow 14y ago
    That might be an improvement, but you probably wouldn't want to use Array.map, which generally has pretty terrible performance. See http://jsperf.com/map-vs-native-for-loop/5 http://jsperf.com/map-vs-native-for-loop/5
    - rb12345 14y ago
      Yes; the only reason for using Array.map() here is conciseness and similarity to the original code.

daeken 14y ago

He's not just evalling code, but building a function without loops and lots of lookups. That's not something you can generalize.

mistercow 14y ago

>Also, does anyone know why eval is faster than just defining the function? I mean wtf, why doesn't v8 make this optimization? There are three optimizations I see in the eval example: 1. The object is created as a literal rather than by property assignment (see http://jsperf.com/object-literals-vs-object-properties http://jsperf.com/object-literals-vs-object-properties). 2. The for loop loop is unrolled. 3. The column names are cached in the function and do not have to be retrieved each time. The reason these can't be done by v8 on its own is that they make assumptions that v8 can't guarantee. For the first two, you have to guarantee that the columns array is always the same length. For the first and last, you have to guarantee that the columns will always have the same names. So even though you take a big hit on the initial eval of the function, you'll likely make up for it by having a better-optimized function.

rb12345 14y ago
I noticed a possible issue with the code generation, which may cause the issues with excessive heap allocation that were reported: var code = 'return {\n'; columns.forEach(function(column) { code += '"' + column.name + '":' + 'parser.readColumnValue(),\n'; }); code += '};\n'; var parseRow = new Function('columns', 'parser', code); Instead of performing so many appends to a single string, I would try using map() and then join() the resulting strings: var code = ['return {\n', columns.map(function(column) { return '"' + column.name + '":' + 'parser.readColumnValue(),\n'; }).join(''), '};\n'].join(''); var parseRow = new Function('columns', 'parser', code); Of course, this depends on how often the parseRow function is created...
- mistercow 14y ago
  That might be an improvement, but you probably wouldn't want to use Array.map, which generally has pretty terrible performance. See http://jsperf.com/map-vs-native-for-loop/5 http://jsperf.com/map-vs-native-for-loop/5
  - rb12345 14y ago
    Yes; the only reason for using Array.map() here is conciseness and similarity to the original code.