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

Fix lab build prod #6907

Merged
merged 4 commits into from Aug 1, 2019
Merged

Fix lab build prod #6907

merged 4 commits into from Aug 1, 2019

Conversation

@telamonian
Copy link
Member

@telamonian telamonian commented Jul 26, 2019

Quoting @tibdex from #6839:

References

Code changes

The changes are small an explained in comments and in the commit messages.

User-facing changes

End-users will have a better UX since they will be running optimized code.

Backwards-incompatible changes

There should be none as switching to prod build by default is non breaking and Webpack is an implementation detail.

@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented Jul 26, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@telamonian
Copy link
Member Author

@telamonian telamonian commented Jul 26, 2019

I added a couple of commits to this PR that:

  • add minimize as an option for lab build.
  • defaults the lab build to dev if there's anything in the "linked_packages" or "local_extensions" fields of build_config.json, and to prod otherwise.

@telamonian
Copy link
Member Author

@telamonian telamonian commented Jul 26, 2019

Quoting @blink1073:

After a rebase I think the logic looks good. Let’s wait until master is at 1.1dev to merge since this is a cli change

@telamonian
Copy link
Member Author

@telamonian telamonian commented Jul 26, 2019

Rebase is done, this is ready to go when 1.1dev is created.

Copy link
Member

@blink1073 blink1073 left a comment

Thanks!

yarn.lock Outdated Show resolved Hide resolved
tibdex and others added 3 commits Jul 31, 2019
With the `TerserPlugin` enabled, the build fails with:

```
~/.local/share/virtualenvs/project-QesvCi_o/share/jupyter/lab/staging
❯ node yarn.js webpack --config webpack.prod.config.js --progress
yarn run v1.15.2
$ /Users/me/.local/share/virtualenvs/project-QesvCi_o/share/jupyter/lab/staging/node_modules/.bin/webpack --config webpack.prod.config.js --progress
 97% [0] chunk asset optimization TerserPlugin
<--- Last few GCs --->

[88233:0x108000000]    26766 ms: Scavenge 1392.5 (1420.9) -> 1391.9 (1421.9) MB, 2.2 / 0.0 ms  (average mu = 0.067, current mu = 0.029) allocation failure

<--- JS stacktrace --->

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

    0: ExitFrame [pc: 0x2b3e51fdbe3d]
Security context: 0x1b222981e6e9 <JSObject>
    1: _walk [0x1b2248a66fc1] [/Users/me/.local/share/virtualenvs/project-QesvCi_o/share/jupyter/lab/staging/node_modules/terser/dist/bundle.min.js:~1] [pc=0x2b3e5223920d](this=0x1b225aef3559 <AST_String map = 0x1b22fdfd8419>,e=0x1b22c00c09c1 <En map = 0x1b22fdfe44f1>)
    2: /* anonymous */ [0x1b22d2ca1001] [/Users/me/.local/share/virtualenvs/chouke...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0x10003cf99 node::Abort() [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 2: 0x10003d1a3 node::OnFatalError(char const*, char const*) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 3: 0x1001b7835 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 4: 0x100585682 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 5: 0x100588155 v8::internal::Heap::CheckIneffectiveMarkCompact(unsigned long, double) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 6: 0x100583fff v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 7: 0x1005821d4 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 8: 0x10058ea6c v8::internal::Heap::AllocateRawWithLigthRetry(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
 9: 0x10058eaef v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
10: 0x10055e434 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
11: 0x1007e6714 v8::internal::Runtime_AllocateInNewSpace(int, v8::internal::Object**, v8::internal::Isolate*) [/Users/me/.asdf/installs/nodejs/10.16.0/bin/node]
12: 0x2b3e51fdbe3d
13: 0x2b3e5223920d
14: 0x2b3e51f918d5
15: 0x2b3e5223a1f7
```

Even on powerful laptops such as a MacBook Pro with a 2,3 GHz Intel Core i9 and 32 GB 2400 MHz DDR4.

The Node.js process would never exit and thus commands like:

```
jupyter labextension install @jupyterlab/plotly-extension --dev-build False
```

would never exit too but without telling why (see jupyterlab#4276).

I've tried a bunch of different terser options but I could not find a combination that would succeed when installing relatively big JupyterLab extensions.
So I resort to disabling minification altogether. I think it's better to have a fat production build than no production build at all.
Now that the production build is fixed, I think it's better to default to it. Indeed, we want users installing JupyterLab to have the best experience using the interface and it means running the optimized production build.

This is especially true for extensions built with React because in dev mode, they will use the React's development bundle which is known to be much slower than the production one.

See also: jupyterlab#6804 (comment)
@telamonian telamonian force-pushed the fix-lab-build-prod branch from bd38818 to cdac8d3 Jul 31, 2019
…xtensions

defaults to prod build otherwise
@telamonian telamonian force-pushed the fix-lab-build-prod branch from cdac8d3 to f7f3fb8 Jul 31, 2019
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jul 31, 2019

@blink1073 - merge if you think this is ready to be merged

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 1, 2019

In it goes!

@blink1073 blink1073 merged commit bdf6136 into jupyterlab:master Aug 1, 2019
9 checks passed
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 15, 2019

Should we backport this to 1.0.x? In the end, did it break any apis that someone may be using developing an extension?

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 15, 2019

We changed the default CLI behavior, which could break some builds.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 15, 2019

So it's a breaking change, as in 2.0? How would a build break?

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 15, 2019

Running out of memory because it would now use the Uglify plugin.

@telamonian
Copy link
Member Author

@telamonian telamonian commented Aug 16, 2019

There are definitely users who will have to adjust their build commands. For example, this guy is an example of user who would probably now need to do jupyter lab build --minimize=False.

It seems that the users who will run into trouble are those with the special circumstance of trying to build Jlab in a resource-constrained VM. Currently, they're essentially relying on an unintended behavior of jupyter lab build (it will always do a dev build, even for end users, and the dev build just so happens to not use the memory-intensive minifier).

However, unlike a breaking change to an API, there will be an easy way for the affected end users to fix their own problem (ie by adding the --minimize=False arg). So I think the best solution would be to add an informative message when the build fails. The message would explain the --dev-build and --minimize args, and also explain how to add them to their config files in order to ensure that their builds always work correctly.

I doubt that we can reliably catch all flavors of out of memory errors, so we could maybe just have that message displayed whenever the build fails for whatever reason.

@vidartf
Copy link
Member

@vidartf vidartf commented Aug 16, 2019

Running out of memory because it would now use the Uglify plugin.

*Terser plugin, right?

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 16, 2019

👍 for a backport after adding the informative message.

*Terser plugin, right?

Yeah, that one 😉

@telamonian
Copy link
Member Author

@telamonian telamonian commented Aug 16, 2019

I took a stab at adding the troubleshooting message in #7037. The PR probably isn't ready yet, and could definitely use some feedback.

@blink1073 blink1073 added this to the 1.1 milestone Aug 22, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants