micromatch / braces

Faster brace expansion for node.js. Besides being faster, braces is not subject to DoS attacks like minimatch, is more accurate, and has more complete support for Bash 4.3.
https://github.com/jonschlinkert
MIT License
220 stars 61 forks source link

Request: Remove or make optional result property on output array #17

Closed BTMorton closed 5 years ago

BTMorton commented 6 years ago

Currently, after doing an expansion, you attach the result object returned from the compiler on the array object. This then gets put into the cache object which takes up a significant amount of memory, particularly when the cache grows large, as it creates a new compiler for each call which is cached, leading to a ~5/6mb overhead every time.

This result property does not appear to be documented behaviour and after a quick github code search, I couldn't find any references to it. Could you either remove this property, make it an optional addition, or suggest a third option to help mitigate the memory usage?

I am currently using chokidar to watch media files for changes, on a folder with ~50k items in it. This leads to the cache object holding ~300mb of memory at ~60% of my total application's memory usage.

Example program to try to demonstrate the issue:

var chokidar = require('chokidar');
var braces = require('braces');

console.log("Test starting");
chokidar.watch('some/folder/with/many/files/*', {}).on('ready', (event, path) => {
  console.log("chokidar ready");

  if (global.gc) { global.gc(); }
  let memoryUsage = process.memoryUsage();
  console.log(`Before clear: ${memoryUsage.heapUsed}/${memoryUsage.heapTotal}`);

  braces.clearCache();

  if (global.gc) { global.gc(); }
  memoryUsage = process.memoryUsage();
  console.log(`After clear: ${memoryUsage.heapUsed}/${memoryUsage.heapTotal}`);

  process.exit();
});

Running the above using node --expose-gc bracesTest.js on my testing file system gives the following output:

Test starting
chokidar ready
Before clear: 446778640/585011200
After clear: 133833408/241065984

My testing system is created using symlinked media files into a single folder with the following script:

#!/bin/bash

for i in {0..50000} 
do
    ln -s ../MediaFile.mp4 clip_$i.mp4
done

Screenshot of the memory snapshot that led me to the cache: image

Screenshot of the memory snapshot after adding a .slice() call to the memoize method when adding an item to the cache: image

jonschlinkert commented 6 years ago

Thank you for taking the time to do this! I just made some major changes this past week that partially address this, but I will also go a step further and make sure that it's documented and that the object is only added if specified by the user for debugging.

jonschlinkert commented 5 years ago

done, braces was refactored and no extra properties are added to the result array.