mcollina / bloomrun

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

Memory leak when running bench2 #59

Closed StarpTech closed 7 years ago

StarpTech commented 7 years ago

Already mentioned in PR #58 It runs out of memory when I run the bench2.js on Node 6,8 but it works on Node 4

> 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

When I copy the pattern on every cycle the error does not appear.

const Benchmark = require('benchmark')
const suite = new Benchmark.Suite()
const bloomrun = require('..')()

suite.add('bloomrun#add', () => {
  bloomrun.add(Object.assign({}, {test: 'pattern'}), 'test pattern')
})
suite.on('cycle', event => console.log(String(event.target))).run()
mcollina commented 7 years ago

Is this happening only with your fix, or in some other case as well?

StarpTech commented 7 years ago

It happens also without my fix.

mcollina commented 7 years ago

It's not happening on Mac OS X on master. It might be Windows related.

StarpTech commented 7 years ago

@mcollina any idea how to fix this? Should we open an issue in NodeJs ?

mcollina commented 7 years ago

I would not worry too much about this benchmark. We are adding an unspecified number of objects to bloomrun, and this might be too much for it. A better approach to verify find, lookup and list is to add a limited amount (say 10000) of patterns, and then run the benchmarks.

StarpTech commented 7 years ago

This not critical but preventing us to bench on window systems and this is not great and the fact that it behaves differently on Windows and Mac.