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

Reuse JSDOM instances across targets #164

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Commits on Jun 18, 2020

  1. Reuse JSDOM instances across targets

    I've been investigating a performance problem we ran into at Airbnb
    the last couple of days. We have some components that end up with
    very very large dependency graphs, which makes the webpack bundle
    quite large. Looking at the CI output, it seems to take about 2 min
    to bundle, which isn't the most terrible thing. But, then I've
    noticed that the timings for the targets are pretty crazy. Look at
    these logs:
    
    ```
    [2020-06-17T19:36:15Z] image_analytics was not bootstrapped
    [2020-06-17T19:36:37Z]   - chrome-medium ✓ (22034.4ms)
    [2020-06-17T19:38:38Z] No examples found for target chrome-mediumPlus, skipping
    [2020-06-17T19:38:38Z]   - chrome-mediumPlus ✓ (4.0ms)
    [2020-06-17T19:41:05Z] p3_defer_modals was not bootstrapped
    ```
    
    Here chrome-medium had examples, so it took snapshots and that took
    a while which makes sense. mediumPlus didn't have examples, so it
    took no snapshots. Happo says that mediumPlus took 4ms, but if you
    look at the timestamps in the log lines, you can see that it
    actually took 2 minutes.
    
    I believe that the time spent on empty targets is in initializing
    the JSDOM and loading up the webpack bundle. Since we have 6
    targets, this adds up to 12 minutes of overhead per job just for
    loading the bundle, and another 12 if we have to check out previous
    SHA. This is much too long.
    
    I think we can improve performance by reusing the JSDOM instance for
    all of the targets. In the example above, this should save us about
    10 minutes on a run where the previous SHA report exists, and 20
    minutes on a run where the previous SHA report does not exist.
    
    One potential issue that could arise from this is if there is any
    side-effecty code in the bundle that looks at the viewport size when
    it is first executed, that will no longer work quite right since we
    run the bundle and then resize the window later.
    
    In order to preserve backwards compatibility with other DomProvider
    plugins, like the puppeteer plugin, I left in the width/height in
    the initialization call, and included a guard around the resize call
    in case it does not exist yet. We should clean these up in the next
    breaking change.
    lencioni committed Jun 18, 2020
    Configuration menu
    Copy the full SHA
    48cf902 View commit details
    Browse the repository at this point in the history
  2. Resolve remaining issues with a reused jsdom

    A couple of issues are fixed in this commit:
    - we can only load the webpack bundle once, so a check for if the
      happoProcessor is initialized already will prevent infinitely waiting
      for the init promise.
    - we need to reset the happoProcessor in between runs, otherwise it
      won't start back at the top.
    - we can't close the jsdom window when we're done. Leaving this as a
      no-op for now, I think this is fine.
    - expecting the resize call to be async will help make it easier to
      update happo-plugin-puppeteer.
    - the init call needs to happen before the resize, now that we reset the
      happoProcessor as part of the resize.
    trotzig committed Jun 18, 2020
    Configuration menu
    Copy the full SHA
    114c08f View commit details
    Browse the repository at this point in the history
  3. Reset cursor in processor constructor

    I removed this in a previous commit, but forgot that
    happo-plugin-puppeteer isn't calling `happoProcessor.reset()`. Having it
    in the constructor as well ensures that the iteration works in the
    puppeteer case as well.
    trotzig committed Jun 18, 2020
    Configuration menu
    Copy the full SHA
    853e4fd View commit details
    Browse the repository at this point in the history
  4. 5.7.0-rc.1

    trotzig committed Jun 18, 2020
    Configuration menu
    Copy the full SHA
    c3fa907 View commit details
    Browse the repository at this point in the history