search

mcollina / bloomrun

A js pattern matcher
MIT License
120 stars 10 forks source link

Feature/#57 max call stack after removing #58

Closed StarpTech closed 7 years ago

StarpTech commented 7 years ago

This fix looks strange to me because I don't know why you add the patternSet after we removed them but this will fix #57 and pass the tests. I found a different issue which is reproducible with and without these changes. If you run npm run bench the bloomrun benchmark will run into a memory leak are you aware of that?

> bloomrun@3.0.5 bench E:\Repositorys\bloomrun
> node benchmarks/bench2.js

patrun#add x 749,526 ops/sec ±0.83% (90 runs sampled)

<--- Last few GCs --->

[8628:000001E2755DF3C0]    17708 ms: Mark-sweep 1257.3 (1314.0) -> 1256.5 (1282.2) MB, 3720.9 / 0.0 ms  last resort
[8628:000001E2755DF3C0]    21439 ms: Mark-sweep 1256.5 (1282.2) -> 1256.4 (1282.2) MB, 3730.7 / 0.0 ms  last resort

<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 000003820D629891 <JS Object>
    2: add [E:\Repositorys\bloomrun\bloomrun.js:~27] [pc=000000CE3CFB55F3](this=000001EAF56DC8F1 <a Bloomrun with map 0000027ABE4DF271>,pattern=0000031FBB9C5E61 <an Object with map 0000027ABE4EC109>,payload=00000059FE7F8441 <String[12]: test pattern>)
    3: fn [E:\Repositorys\bloomrun\benchmarks\bench2.js:~13] [pc=000000CE3CFBBCD4](this=000001EAF56EC6A9 <a Benchmark with map 0000027ABE49AE59...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
StarpTech commented 7 years ago

Looks like a memory leak in node 6, 8 (tested) because in node 4 it works.

StarpTech commented 7 years ago

Closed due to https://github.com/mcollina/bloomrun/pull/60