Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaScript heap out of memory #637

Closed
bignass04 opened this issue Mar 6, 2017 · 12 comments
Closed

JavaScript heap out of memory #637

bignass04 opened this issue Mar 6, 2017 · 12 comments

Comments

@bignass04
Copy link

I am using Pattern lab Node v2.8.0 on Mac with Node v6.9.1.

ERROR

I get the following error when my app is building the handlebars patterns:
Security context: 0x81bb64cfb51
2: string(aka string) [/Users/mnasser/ha50/node_modules/json5/lib/json5.js:~218] [pc=0x1dd9da48b83e] (this=0x81bb6404381 )
3: object(aka object) [/Users/mnasser/ha50/node_modules/json5/lib/json5.js:~433] [pc=0x1dd9da48ee26] (this=0x81bb6404381 )
4: object(aka object) [/Users/mnasser/ha50/node_modules/json5/lib/json5.js:~433] [pc=0x1dd9da48eed4] (this=0x81bb64043...
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
1: node::Abort() [/Users/mnasser/.nvm/versions/node/v6.9.1/bin/node]
2: node::FatalException(v8::Isolate, v8::Localv8::Value, v8::Localv8::Message) [/Users/mnasser/.nvm/versions/node/v6.9.1/bin/node]
3: v8::internal::V8::FatalProcessOutOfMemory(char const, bool) [/Users/mnasser/.nvm/versions/node/v6.9.1/bin/node]
4: v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/Users/mnasser/.nvm/versions/node/v6.9.1/bin/node]
5: v8::internal::Runtime_AllocateInTargetSpace(int, v8::internal::Object*, v8::internal::Isolate) [/Users/mnasser/.nvm/versions/node/v6.9.1/bin/node]
6: 0x1dd9d9f079a7
sh: line 1: 97019 Abort trap: 6 node scripts/patternlab.js build

RESOLUTION

I found that I can resolve the error if I go to patternlab-node/core/lib/patternlab.js and change line 428 from
allFooterData = JSON5.parse(JSON5.stringify(patternlab.data));
to
allFooterData = JSON.parse(JSON.stringify(patternlab.data));

and change line 373 from
allData = JSON5.parse(JSON5.stringify(patternlab.data));
to
allData = JSON.parse(JSON.stringify(patternlab.data));

BENCHMARKS

The build times were recorded with one time stamp displayed immediately after line 32 of patternlab-node/core/lib/patternlab.js as the start time and one immediately after the build process is completed as the end time.

  • Number of patterns: 76
  • Build time with JSON5 (in seconds): 35.3
  • Build time with JSON (in seconds): 7.08
@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Mar 7, 2017

Additional info.

JSON5 was added with #293
Coincidentally this is also when #316 was first reported. I want to look into this angle a bit more, and think that we can likely support both via configuration. (Speed vs listed json parsing)

I also recall that handlebars engine might have a bigger object structure than others, but I think this is still a valid thread to tug at.

cc @bramsmulders

@bmuenzenmeyer bmuenzenmeyer self-assigned this Mar 7, 2017
@bmuenzenmeyer
Copy link
Member

my own benchmarking:

node: 6.9.5
PL Node 2.8.0
data: 68 lines <--- almost none
patterns: 120 mixed between mustache and handlebars
build time with JSON5 (in seconds): 4.36s
build time with JSON (in seconds): 3.64s (17% faster)
adding more data, say, by including the starterkit-mustache-demo json, did not have an appreciable difference.

@bignass04, what is the total size of your data.json file, or your overall data footprint? do you have lots of pattern-specific data, for instance?

@simonknittel
Copy link

simonknittel commented Mar 10, 2017

We did a quick fix for our CI servers by creating an file named fix-build.sh containing:

echo $(uname)

if [ "$(uname)" = 'Linux' ]; then
  echo 'Linux detected.'
  sed -i "s/allFooterData = JSON5.parse(JSON5.stringify(patternlab.data));/allFooterData = JSON.parse(JSON.stringify(patternlab.data));/g" node_modules/patternlab-node/core/lib/patternlab.js
  sed -i "s/allData = JSON5.parse(JSON5.stringify(patternlab.data));/allData = JSON.parse(JSON.stringify(patternlab.data));/g" node_modules/patternlab-node/core/lib/patternlab.js
elif [ "$(uname)" = 'Darwin' ]; then
  echo 'macOS detected.'
  sed -i "" "s/allFooterData = JSON5.parse(JSON5.stringify(patternlab.data));/allFooterData = JSON.parse(JSON.stringify(patternlab.data));/g" node_modules/patternlab-node/core/lib/patternlab.js
  sed -i "" "s/allData = JSON5.parse(JSON5.stringify(patternlab.data));/allData = JSON.parse(JSON.stringify(patternlab.data));/g" node_modules/patternlab-node/core/lib/patternlab.js
fi

And then adding it to the postinstall script of npm: "postinstall": "sh fix-build.sh"

@bmuenzenmeyer
Copy link
Member

Wow that's a decently involved fix - I will prioritize a better solution.

@bignass04
Copy link
Author

@bmuenzenmeyer The total data.json file is 714KB. I don't think we have much pattern-specific data but I haven't done much work on this application yet.

@bmuenzenmeyer
Copy link
Member

for reference, mine above was 2KB

Quite the difference.

@simonknittel
Copy link

Our data.json is currently at 78KB, but with 440 individual/pattern specific JSON files.

@bmuenzenmeyer
Copy link
Member

I've generated some test data and will be using that for further analysis

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Mar 21, 2017

Testing:


  1. JSON5.parse(JSON5.stringify(patternlab.data));

    gulp patternlab:build
    completed in 34 seconds

  2. JSON.parse(JSON.stringify(patternlab.data));

    gulp patternlab:build
    completed in 8.68 seconds

  3. jsonCopy(patternlab.data, 'config.paths.source.data'); <-- new helper file, which lints the JSON and then uses lodash cloneDeep

    gulp patternlab:build
    completed in 11 seconds

  4. jsonCopy(patternlab.data, 'config.paths.source.data'); <-- new helper file, only uses lodash cloneDeep, does not lint (which could allow malformed JSON into rendering)

    gulp patternlab:build
    completed in 10 seconds

@bmuenzenmeyer
Copy link
Member

I am going with a pure JSON.stringify(JSON.parse(data)) implementation, and going to retain the try catch for other applications as they may arise. proper linting should occur at time of read, not copy.

bmuenzenmeyer added a commit that referenced this issue Mar 21, 2017
bmuenzenmeyer added a commit that referenced this issue Mar 21, 2017
we will error handle json during load after #634 solidifies
part of #637
bmuenzenmeyer added a commit that referenced this issue Mar 21, 2017
…w helper

we will error handle json during load after #634 solidifies
part of #637
@bmuenzenmeyer
Copy link
Member

I think I have fixed this with PL Node 2.9.X

@bmuenzenmeyer
Copy link
Member

resolved (i think) with 2.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants