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

Precompile Renderers with Prepack #12206

Closed
gaearon opened this issue Feb 10, 2018 · 7 comments
Closed

Precompile Renderers with Prepack #12206

gaearon opened this issue Feb 10, 2018 · 7 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Feb 10, 2018

Filing this for future work. (If anyone wants to take this up, feel free to! But it's not the easiest task and we won't be able to answer a lot of questions so you'll be largely on your own. It's fun though!)

We want to remove as much indirection as we can in hot paths. However React reconciler is abstracted away from the underlying platform through a renderer "host config". For example, ReactDOM says "here's how you insert a DOM node", and then the reconciler uses this function. The problem is that we get that function by passing a "config" object around. This is both extra code to have around, and extra indirection at runtime.

Note this approach will not significantly reduce the code size. But I hope it could make the runtime a bit more efficient.

Ideally we want the compiled reconciler code to directly include calls into the DOM APIs. We already use Closure Compiler which helps with inlining. But it's not smart enough to see that all functions in the "host config" can be fully inlined because the object itself never escapes the bundle. Prepack is smart enough for this because it actually executes the initialization code.

With this hypothetical new approach, we run Prepack first to get rid of those intermediate representations. Then we run Closure Compiler on top of that. Less indirection means Closure Compiler can be smarter about what to inline. I made a proof of concept that shows that the DOM methods successfully end up right in the reconciler hot paths, just like we want:

screen shot 2018-02-10 at 18 39 38

My proof of concept was hacky and done outside the build workflow so I won't be sending a PR. But here's roughly what I did (and what you can try).

First, you'll need to add Prepack to our build workflow. #11716 roughly shows where that should be done, even though this will give you a broken bundle. You'll need to make sure you include simpleClosures: true and (maybe?) omitInvariants: true in the options. It will still be broken.

The main reason it breaks immediately is the UMD wrapper emitted by Rollup. Prepack just doesn't understand what that soup of checks with exports, module, and require means. I don't think there's an easy way to model this without changing Prepack so I found it easier to change this function to return 'iife'. This will tell Rollup to output a simple "factory pattern" that assigns to a global, which is more than enough to start hacking on this (but of course will only work in a browser).

Prepack will still be confused by a few things. I found it easiest to just take the Rollup bundle, save it on the disk, and then manually tweak it and re-run Prepack CLI on it afterwards so that I could quickly get an idea for what exactly is breaking.

There are a few common cases:

  • Access to React.* (because Prepack doesn't know what it is).

    • It's probably possible to get around this by modeling React as an abstract global, i.e. something like

      React = __abstract({
        __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: __abstract({
          ReactCurrentOwner: __abstract({
            current: __abstract('object')
          }),
          // TODO: other things?
        })
      }, 'React');
      __makeSimple(React);
      __makeSimple(React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED);
      __makeSimple(React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentOwner);

    prepended to the input bundle.

    I didn't get very far there so I just replaced React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.assign with Object.assign in the bundle, but we'll need a proper fix.

  • Access to window and document during initialization.

    • Some cases like document.documentMode can be shimmed with something like:
    document = __abstract({
      documentMode: __abstractOrUndefined('number'),
    });
    __makeSimple(document);
    • Other cases like document.createElement('div').style probably won't work at all. I don't know how to model this as abstract.

    • For all cases that aren't clear, I just made them initialize lazily. Prepack only executes the initial path. So code like:

    var supportSomeFeature = document.documentMode <= 11;
    
    function shouldDoSomething() {
      if (supportSomeFeature) {
        // ...
      }
    }

    could be written like:

    var didInit = false;
    var supportSomeFeature;
    
    function ensureReady() {
      if (didInit) {
        return;
      }
      didInit = true;
    
      // go wild here!
      supportSomeFeature = document.documentMode <= 11;
    }
    
    function shouldDoSomething() {
      ensureReady();
      if (supportSomeFeature) {
        // ...
      }
    }

    I'm not super happy about this but it works. There's like 4 or 5 places where this ends up being necessary. Maybe we can model more with abstracts but I don't know Prepack that well.

  • Non-deterministic calls like Math.random().toString()

    Not sure what to do about those. For now I just shimmed them to be string literals. Maybe we can teach Prepack that Math.random().toString() is also an abstract string somehow.

Say we just fix all these immediate issues. Don’t get too excited! The first bundle will be huge. This is because Prepack pre-evaluates all metaprogramming. So code like this will turn into giant "precomputed" object literals with all the final values.

One way to solve it is just to remove code like this. It can be tricky although with time we should move into that direction. Another way is to delay initialization. Prepack only "pre-executes" the initial path. So we can change the code to lazily initialize those objects (e.g. not until the functions that use them are called for the first time). I already did some work to find those places:

These are the cases that Prepack "explodes". By solving them you'll get back to roughly the same bundle size as normal React. For my super hacky version, I made all event plugins "lazy" and injected them during the first ReactDOM.render call. This way Prepack doesn't attempt to pre-evaluate those paths (which contains those explosive objects). Maybe there's a better way (e.g. making just eventTypes lazy, or somehow avoiding the need for those objects altogether).

To get rid of the host config, I needed to make sure parts of it like mutation object aren't not referenced in inner closures. Otherwise Prepack won't know it's safe to omit it. So I added hasMutation = config.mutation early and then used just that (and the destructured functions themselves). By that point my bundle was already a few hundred bytes smaller than the original one, and the host config indirection was gone.

The final problem I bumped into was that GCC was running out of memory. For some reason Prepacked input puts it under more pressure. I was able to work around by manually bumping the process memory limit:

node --max-old-space-size=8192 ./scripts/rollup/build.js dom-client --type=UMD_PROD 

It peaks around 5GB but then compiles.

To sum up, this was fun as an evening hack, and is probably a viable longer term strategy. We need to think about how to solve those object initialization issues and whether we want to make more things lazy. Of course we also need to figure out how to integrate this into our build properly (e.g. do we just re-add UMD/Node headers later? do we model them with Prepack?)

I think it's a fun task to work on for motivated contributors so I'll tag it as one. But please don't expect that the result would get merged. This is mostly exploratory work. It would be awesome to see a working PR though!

@gaearon
Copy link
Collaborator Author

gaearon commented Feb 10, 2018

cc @banga in case you find this interesting

@Velveeta
Copy link

I know you all probably take this very seriously internally, and with good reason, but every time I see _DO_NOT_USE_OR_YOU_WILL_BE_FIRED, like back before you had separated react-dom out from the core library, I can't help but chuckle a little bit inside :)

@trueadm
Copy link
Contributor

trueadm commented Feb 11, 2018

It might also be worth ensuring the right compatibility mode is enabled as an option passed to Prepack. If you enable browser, then that comes with a bunch of predefined abstracts for the DOM and things like document. More need to be added though.

@gaearon
Copy link
Collaborator Author

gaearon commented Feb 11, 2018

@trueadm Strangely, I tried that and it made no difference. Maybe there's just too little there.

@trueadm
Copy link
Contributor

trueadm commented Feb 11, 2018

@gaearon There's not a huge amount: https://github.com/facebook/prepack/blob/master/src/intrinsics/dom/global.js#L21-L118

But some key ones, like setTimeout, document.createElement etc are supported.

@banga
Copy link

banga commented Apr 2, 2018

(Finally caught up to this after a long vacation) This sounds really interesting. I was a little surprised that GCC wasn't inlining at least some of these functions. I tried a minimal example here and it looks like GCC never inlines methods when passed via a config object like React does, despite all of my attempts to indicate that the object will never change. I'm not familiar with Prepack but it sounds exciting that it get can get around whatever is stopping React.

@gaearon
Copy link
Collaborator Author

gaearon commented May 21, 2018

We inlined host configs with #12792 so this probably won't have as much valuable impact anymore.

@gaearon gaearon closed this as completed May 21, 2018
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

4 participants