fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.89k stars 296 forks source link

Rollup failing when building fable-standalone #2987

Closed alfonsogarciacaro closed 2 years ago

alfonsogarciacaro commented 2 years ago

I was trying to update the REPL but it seems we were very close to Rollup's limits because after the latest commits I'm getting "JavaScript heap out of memory" errors when generating the bundle.

I've tried to increment Node's memory as recommended here (I tried both the way mentioned in the link and also setting the NODE_OPTIONS env var). But I'm still getting the same error in my computer :/

alexswan10k commented 2 years ago

This is a super common problem in the node world. Basically half the commercial webpack projects i have worked on have failed with this at some point.

The solution is absolutely correct, you may just not be invoking it correctly. Where is the script exactly? Maybe i can help.

alfonsogarciacaro commented 2 years ago

Thanks for your help @alexswan10k! It's this line: https://github.com/fable-compiler/Fable/blob/adc626d63451047be23af05f8d8b2010c23f07c7/build.fsx#L353

I tried adding the following just on top but it didn't seem to work :/

Environment.SetEnvironmentVariable("NODE_OPTIONS", "--max-old-space-size=8192")

If we don't manage to make it work, maybe we can consider switching to esbuild. I got great results with it in another project.

alexswan10k commented 2 years ago

Is this on snake_island?

I am trying to replicate now, but usually the simplest dumbest way to do this is to just swap out the root executable with a node placeholder, and supply this as parameters. For example, rather than this:

npm run yourscript

you might do

node --max_old_space_size=8000 /usr/bin/npm run yourscript 

Alternatively I think you can do stuff like this

$ npm run start --node-flags --max-old-space-size=512 --no-warnings

Is it just via npm this is being invoked?

There are a whole bunch of workarounds here too https://github.com/npm/npm/issues/12238

Your environment variable approach should work too though in theory...

alexswan10k commented 2 years ago

This seems to work for me:

Try replacing line 73 build.fsx with this

    let runNpmScript script args =
        run ("npm run " + script + " --node-flags --max-old-space-size=8192" + " -- " + (String.concat " " args))
alfonsogarciacaro commented 2 years ago

Thank you @alexswan10k! This is strange, it's not working for me even when raising the memory to 16384. Maybe it's an issue with my machine?

image

alexswan10k commented 2 years ago

You are correct this does not work, sorry for leading you up the garden path! I had something weird going on with my environment which made it appear to be working fine. Trying some other stuff locally..

alexswan10k commented 2 years ago

Hmm something weird is going on

image

In short I think the switch is working but rollup has gone on an allocation binge. So I have now just added this to the top of build.fsx as you originally pointed out

Environment.SetEnvironmentVariable("NODE_OPTIONS", "--max-old-space-size=16000")

Eventually I still ran out image image

Ideas https://github.com/rollup/rollup/issues/2377

Going to try with even more! Lets try 24gb

edit 24gb did not cut it

ncave commented 2 years ago

I can replicate the issue with fable-standalone on my machine, it has nothing to do with rollup. What happens is Fable CLI being stuck on compiling src/Fable.Transforms/Fable2Rust.fs and generating invalid code.

If you have noticed, it compiles every other file in fable-standalone, then it just sits there for another 4-5 minutes doing nothing. Eventually it finishes, but the file it produces is invalid, so rollup chokes afterwards.

If you look at build/fable-standalone/bundle/Fable.Transforms/Rust/Fable2Rust.js and scroll down, you'll see the problem. Here is the error that it spits over and over:

        throw new Error("Cannot find IEnumerator interface, should not happen.");
    })())))) : ((typ.tag === 16) ? ((activePatternResult_11 = $007CBuiltinEntity$007C_$007C(typ.fields[0].FullName, typ.fields[1]), (activePatternResult_11 != null) ? ((kind_1 = activePatternResult_11, (kind_1.tag === 1) ? TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]) : ((kind_1.tag === 2) ? TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]) : ((kind_1.tag === 3) ? TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]) : ((kind_1.tag === 4) ? TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]) : ((kind_1.tag === 5) ? TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]) : ((kind_1.tag === 6) ? TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]) : ((kind_1.tag === 7) ? TypeInfo_transformHashSetType(com, ctx, kind_1.fields[0]) : ((kind_1.tag === 8) ? TypeInfo_transformHashMapType(com, ctx, ofArray([kind_1.fields[0], kind_1.fields[1]])) : ((kind_1.tag === 10) ? TypeInfo_transformSetType(com, ctx, kind_1.fields[0]) : ((kind_1.tag === 11) ? TypeInfo_transformMapType(com, ctx, ofArray([kind_1.fields[0], kind_1.fields[1]])) : ((kind_1.tag === 9) ? TypeInfo_transformTupleType(com, ctx, true, ofArray([kind_1.fields[0], kind_1.fields[1]])) : ((kind_1.tag === 13) ? TypeInfo_transformResultType(com, ctx, ofArray([kind_1.fields[0], kind_1.fields[1]])) : ((kind_1.tag === 12) ? TypeInfo_transformChoiceType(com, ctx, kind_1.fields[0]) : ((kind_1.tag === 14) ? ((activePatternResult_12 = $007CIsInRefType$007C_$007C(com, typ), (activePatternResult_12 != null) ? ((genArg_7 = activePatternResult_12, TypeInfo_transformType(com, ctx, genArg_7))) : TypeInfo_transformRefCellType(com, ctx, kind_1.fields[0]))) : TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1])))))))))))))))) : TypeInfo_transformDeclaredType(com, ctx, typ.fields[0], typ.fields[1]))) : (() => {
        throw new MatchFailureException("/home/dev/Projects/Fable/src/Fable.Transforms/Rust/Fable2Rust.fs", 738, 18);
    })())))) : ((activePatternResult_10 = $007CIsEnumerator$007C_$007C(typ), (activePatternResult_10 != null) ? ((entRef_6 = activePatternResult_10[0], (genArgs_2 = activePatternResult_10[1], (matchValue = TypeInfo_tryFindInterface(com, "System.Collections.Generic.IEnumerator`1", entRef_6), (matchValue != null) ? ((ifc = matchValue, TypeInfo_transformInterfaceType(com, ctx, ifc.Entity, singleton(TypeInfo_inferredType)))) : (() => {

Looking at the error MatchFailureException("/home/dev/Projects/Fable/src/Fable.Transforms/Rust/Fable2Rust.fs", 738, 18), seems like some kind of match error.

I've noticed this started a to happen a few commit back, you can see since then the JS tests taking longer by 5 min for no good reason (well, actual reason being too many errors are being generated and logged).

The thing is, this doesn't happen when compiling fable-standalone using the bench compiler which uses fcs-fable and its own CLI, instead of Fable.Cli, and it compiles src/Fable.Transforms/Fable2Rust.fs just fine in under a second. You can try it yourself, cd src/fable-standalone/test/bench-compiler && npm run build.

So this seems to be some issue in Fable.Cli, in the sense that perhaps the FCS it uses is subtly different than FCS-Fable, and the F# AST it produces is subtly different? Not sure yet, just wanted to share before diving deeper.

ncave commented 2 years ago

Fixed in #2988. I still don't know why it's happening, we may need to create a small repro. More importantly, why only in FCS but not FCS-Fable? I guess FCS-Fable is just better ;)

How do you make a better FCS, you ask? As the great Michelangelo once said:

“It is easy. You just chip away the code that doesn’t look like FCS.”

ncave commented 2 years ago

Closing as resolved (in #2988).

alfonsogarciacaro commented 2 years ago

Wow, that was a fantastic deep dive @ncave! I could have investigated this for days and still blame Rollup 😅

I've pushed a new release of the repl4 with the latest changes. This one will also save the selected language and a couple of other minor fixes 👍