Closed Bug 991904 Opened 10 years ago Closed 10 years ago

Create a way to lazily load modules with the devtools loader

Categories

(DevTools :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 31

People

(Reporter: irakli, Assigned: fitzgen)

References

Details

Attachments

(1 file, 1 obsolete file)

There has being cocerns in regards to devtools's slow startup due to amount of code being loading. There have being desire to fix that by allowing lazy module loading similar to `XPCOMUtils.defineLazyModuleGetter`.
I'm resistant to add require.lazy("my/modul") style hacks because they imply dynamic scope manipulation that is not very standarty in JS on one hand, and is a poor mans solution on the other.

What I think would be a more appropriate solution that is in synergy with future JS standards and less
confusing for rest of the JS engineers / toolchain is to make require behavior just lazy. Loader could
always return proxy object that would defer actual module loading to a point where imports are actually
accessed. That way all of the code loading will be deferred until it's use. There is also a simple escape
out of lazy loading via use of destructing.

Concern of this approach is that result of lazy loading may have observable effects on a program, but in practice I don't expect that to be a problem as long as module does not do any initialization / side effects in the body itself. In other words as long as your module is just a set of functions you call you
should be just fine, which is a recommended pattern any how.
(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #1)

> In other words as long as your module is just a set of functions you
> call you
> should be just fine, which is a recommended pattern any how.

But currently we do a lot of things in our SDK modules that are not part of any functions. Most of them, actually: define classes, set up listeners… and you're saying that that would cause issues? I'm not sure I'm totally following you.
(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #1)
> I'm resistant to add require.lazy("my/modul") style hacks because they imply
> dynamic scope manipulation that is not very standarty in JS on one hand, and
> is a poor mans solution on the other.

Dude. I've told you like ten times in irc. That's not what I'm talking about. I'm talking about something exactly like https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/XPCOMUtils.jsm#defineLazyModuleGetter%28%29 but for require. No magic scope manipulation.

    require.defineLazyGetter(this, "module", "path/to/my/module");

    function foo() {
      module.whatever();
    }

> 
> What I think would be a more appropriate solution that is in synergy with
> future JS standards and less
> confusing for rest of the JS engineers / toolchain is to make require
> behavior just lazy. Loader could
> always return proxy object that would defer actual module loading to a point
> where imports are actually
> accessed. That way all of the code loading will be deferred until it's use.
> There is also a simple escape
> out of lazy loading via use of destructing.

This is really magical and not explicit. There are plenty of cases where making something lazy doesn't make sense because the module is used immediately. Changing the default require behavior adds overhead.

I would be less against something like

    const lazyProxyToModule = require.lazyProxy("path/to/my/module");

> Concern of this approach is that result of lazy loading may have observable
> effects on a program, but in practice I don't expect that to be a problem as
> long as module does not do any initialization / side effects in the body
> itself. In other words as long as your module is just a set of functions you
> call you
> should be just fine, which is a recommended pattern any how.

It *does* have observable effects, whether it creates bugs is another question.
(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #0)
> There has being cocerns in regards to devtools's slow startup due to amount
> of code being loading. There have being desire to fix that by allowing lazy
> module loading similar to `XPCOMUtils.defineLazyModuleGetter`.

This is more about memory usage from loading compartments that often sit there unused.
Thinking about this some more, to have a loader be lazy without some explicit other method (`require.lazy('module')`) would be a huge difference compared to node's require statement. It's most likely against CommonJS spec (I can't find a definitive answer on this), but in node/browserify environments, loading things into the environment is done, which requires it to be non-lazy:

```
require('./some/thing/to/be/executed');
```

If we want lazy es6 module stuff in the future, fine, but that shouldn't change the semantics of our commonjs module system. Having a `require.lazy` in the meantime seems fine, and will probably only be used for devtools anyway. Pretty much, using the same loading logic as node is a huge benefit and changing that will be a step backwards.
Yeah, I certainly rely n the actual execution of require stuff at import time for things (registering types with protocol.js for example).
Flags: needinfo?(dcamp)
And I agree with nick - in cases where we know it's safe to load something lazily, something like defineLazyGetter is fine, and require.lazyProxy would be fine too.  But changing the semantics of require will break us.
Attached patch lazy-require.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=26adae0c0a9f
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Attachment #8412121 - Flags: review?(dcamp)
Component: General → Developer Tools
Product: Add-on SDK → Firefox
Summary: Make loader load modules lazily → Create a way to lazily load modules with the devtools loader
Attachment #8412121 - Flags: review?(dcamp) → review+
https://hg.mozilla.org/integration/fx-team/rev/7f3e428b4a5d
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
This should fix those failures.

https://tbpl.mozilla.org/?tree=Try&rev=995dd3df35b4
Attachment #8412121 - Attachment is obsolete: true
Attachment #8412775 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/c00e60f19024
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c00e60f19024
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.