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

Bug report: requiring the node module is incredibly slow #1016

Closed
Prinzhorn opened this issue Apr 16, 2020 · 3 comments
Closed

Bug report: requiring the node module is incredibly slow #1016

Prinzhorn opened this issue Apr 16, 2020 · 3 comments
Labels

Comments

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Apr 16, 2020

Describe the bug
require('cyberchef') takes roughly 3s to 4s. Tested on both Node.js 10 and 12. Might be much worse without a fast SSD.

To Reproduce

console.time('require');
require('cyberchef');
console.timeEnd('require');

Expected behaviour
A couple of ms

Desktop (if relevant, please complete the following information):

  • OS: Ubuntu 19.10
  • Browser: Node.js
  • CyberChef version: 9.20.3

Screenshots

Selection_608

Additional context

3s might be somewhat acceptable in a long-running server process, but unfortunately it's beyond acceptable in an Electron app where startup time without cyberchef is otherwise instant.

It appears that a lot of time is spent by "esm.js". I tried compiling a wrapper script using ncc but it fails. It seems like it tries to load some of the web stuff, even if I explicitly require('cyberchef/src/node/index.mjs')

Using import with --experimental-modules doesn't make it fast at all. Edit: correction, explicitly doing import chef from "cyberchef/src/node/index.mjs"; brings it down to like 2s.

Would it be possible to compile a single bundle for node just like it's done for the web version?

@Prinzhorn Prinzhorn added the bug label Apr 16, 2020
@Prinzhorn
Copy link
Contributor Author

I dug into this just a tiny bit (and was looking at #284) and I'm wondering if there are plans to move to a more standard approach to modules? What I mean by that is that this looks like a mix of Grunt, webpack and esm (which seems unmaintained and unnecessary?). I don't even know where to start making sense of everything.

I understand that this was historically a Browser-only application and has then been refactored to also support Node.js. Which explains where this is coming from. But when I'm consuming a package in Node.js, I expect it to use the built-in module system. Right now even the Node.js build relies on webpack and then at runtime it uses a module loader (esm.js), written in JavaScript.

I will look into this again in the future, as I'm at least looking for a workaround to compile a Node.js version that loads in the ms range, preferably single-digit. I will report back once I'm happy with the results.

@d98762625
Copy link
Member

Hello, thanks for your issue. This had prompted me to have a dig through NodeJS docs around ESM/CJS interoperability and hopefully the PR above will help improve things.

As you say CyberChef was written for the web. It's written entirely in ES Modules, which are historically incompatible with CommonJS modules, Node.js's module ecosystem.

Later versions of node are improving interoperability between ESM and CJS, but its not perfect. Using the esm package allows us to export the CyberChef Node API so that it is fully compatible with CJS by default, but as you've raised, that gives a significant performance hit. Thanks for your flame graph by the way, I've tried out something similar in the above PR. We need to use ESM here because the only "native" way to load an ES module from a CJS module is to use import(), which would force every CSJ consumer of CyberChef to load it asynchronously.

Take a look at the above PR, see if it helps your cause. I would recommend trying the import path, as I've used package exports to define different entrypoints for import and require. This comes with the caveat that you'll have to add some flags (documented in the PR) at runtime.

I'd be interested to know if this helps with your performance issues.

@d98762625
Copy link
Member

@Prinzhorn based on the conversation in the PR linked above, I'm going to close this issue. Feel free to re-open it or raise more specific issues if you find any.

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

No branches or pull requests

2 participants