Octane regexp.js does not complete with the right check sum.
Opened this issue · 3 comments
I was trying to look at the performance of this library, by running the octane regexp benchmark.
After doing some modification[1] to the benchmark, such as it use the RegExp constructor instead of the literal RegExp notation. I noticed that at least the first 2 regexps which are involving \s are not working as expected and find more matches than YARR.
[1] sed -ie '
s~= /([^"])/([ig]);= new RegExp("\1", "\2"); ;
sExec(/([^"])/([ig]), ;Exec(new RegExp("\1", "\2"), ~ ;new RegExp("([^"]*)~new RegExp("\1.backslash.
T; :b;
s
t b; s~.backslash.~\~g' regexp.js
Thanks for opening this issue!
Could you please make the JS you test/execute available to me? My sed
understanding is very limited and I am not sure if the issue you describe is due to a bug in RegExp.JS or due to a bug in the sed
commands. Once I can get hold of the JS you execute, I am more than happy to check why the library has more matches than YARR.
Looking at the octane benchmark, I think the problem with the \s
RegExp comes due to this lines:
var re2 = /^\s*|\s*$/g;
[...]
sum += s2[i].replace(re2, '').length;
The RegExp is used as argument to the String.prototype.replace
function. The problem is, that passing a RegExpJS object to the String.prototype.replace
function does not work.
Could this be the root of the problem?
Yes, this sounds like this might be the root of the problem, in which case I guess can replace String.prototype.replace such as it check for RegExpJS.
About the sed expression, this is my mistake, I replaced a non-printable character by the "...." sequence, you can replace this sequence by ":!:!:" and this should work fine.
https://github.com/nbp/regexp.js/tree/octane-benchmarkgit@github.com:nbp/regexp.js.git