bem-xjst: clean cached flags for wrap() and extend() (fix for #495)

[miripiruni]

Fixes #495

Changes proposed in this pull request

On every templates.apply(bemjson) call we can use copy of bemjson, not the bemjson by link.

Checklist

• Tests added
• Benchmark checked

./lib/compare.py ./dat-10550-4ee9b ./dat-10550-4ee9b/10550d5fa9e1cf54edec5b86e30b261b533488c6-2000-1523269017851.dat ./dat-10550-4ee9b/4ee9bbd2059c6e8dc6627785e591bf9fa3a27a06-2000-1523269032181.dat
/bin/sh: gnuplot: command not found
/bin/sh: gnuplot: command not found
Percentile:  0.5
{ rev1: 4.211397,
  rev2: 4.089813,
  'diff abs': 0.12158399999999947,
  'diff percent': 2.8870229997314256 }

Percentile:  0.9
{ rev1: 6.285278,
  rev2: 6.214809,
  'diff abs': 0.07046900000000011,
  'diff percent': 1.1211755470482032 }

Percentile:  0.95
{ rev1: 7.158743,
  rev2: 6.986244,
  'diff abs': 0.17249900000000018,
  'diff percent': 2.4096269414895954 }

@tadatuta if it looks like production decision for you, I can run benchmarks.

[miripiruni]

https://www.traviscistatus.com/incidents/c1s4dlyxd4lf

[coveralls]

Coverage increased (+0.005%) to 99.408% when pulling 3b55c9a on issue-495__isolate-input-bemjson into 4ee9bbd on master.

[miripiruni]

Travis alive.

[miripiruni]

@tadatuta ping

[tadatuta]

I'm not sure that shallow copy is a good fix :(

For example such case is still broken:

const bemhtml = require('bem-xjst').bemhtml;
const tmpl = bemhtml.compile(function() {
    block('b3').wrap()(node => ({
        block: 'b2',
        content: node.ctx
    }));
});

const bemjson = { block: 'b1', content: { block: 'b3' } };

const firstTime = tmpl.apply(bemjson);
const secondTime = tmpl.apply(bemjson);

console.log('firstTime:', firstTime);
console.log('secondTime:', secondTime);

[miripiruni]

@tadatuta OK, that was a quick attempt. The bug is deeply related to https://github.com/bem/bem-xjst/blob/master/lib/bemxjst/match.js#L46-L47 and it’s flag prevent infinity loop. Right now I have no idea how to fix it… :(

Do you have any suggestions?

[tadatuta]

@zxqfox we discussed possible fix with @miripiruni few days ago.
the idea is to collect pointers to nodes where wrap flag was added and remove the flag for each apply() call. but i'm still not quite sure if it's fine (as it won't fix the issue for manual interaction inside def() mode at least).

[miripiruni]

@tadatuta Updated according to your idea.

Benchmark result ~ +2%…

cc @@zxqfox

[tadatuta]

Benchmark result ~ +2%…

2% slower then previous solution with shallow copy?
Looks to much for me.
How many wrap/extend calls are there in bench templates?

[miripiruni]

@tadatuta

How many wrap/extend calls are there in bench templates?

11. (and ~450 block() subpredicates). How many times wrap() was matched I don’t know.

2% slower then previous solution with shallow copy?
Looks to much for me.

measurement error ~1%. It’s normal measurement error on my macbook with turned off network.

After optimisation commit results looks better:

rm -fr dat-10550-4ee9b/ && node runner.js --rev1 https://github.com/bem/bem-xjst/pull/496/commits/285637ced9a5b6590805fb93340ea765e70320d0 --rev2 4ee9bbd2059c6e8dc6627785e591bf9fa3a27a06 --bemjson 2000 --dataPath ~/Documents/www/web-data/data --templatePath ./template-8x.js
Percentile:  0.5
{ rev1: 4.059073,
  rev2: 3.991944,
  'diff abs': 0.06712899999999955,
  'diff percent': 1.6538012496941978 }

Percentile:  0.9
{ rev1: 6.045441,
  rev2: 6.015059,
  'diff abs': 0.030382000000000353,
  'diff percent': 0.5025605245341125 }

Percentile:  0.95
{ rev1: 7.035301,
  rev2: 6.961316,
  'diff abs': 0.07398499999999952,
  'diff percent': 1.0516252254167857 }

I tend to think of this as an insignificant speed change.