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

Embedding loading slow #13472

Closed
flamber opened this issue Oct 14, 2020 · 6 comments
Closed

Embedding loading slow #13472

flamber opened this issue Oct 14, 2020 · 6 comments
Assignees
Labels
Embedding/Static Static embedding, previously known as signed embedding .Performance Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness Type:Bug Product defects
Milestone

Comments

@flamber
Copy link
Contributor

flamber commented Oct 14, 2020

Describe the bug
Loading an embedding is a lot slower on 0.35.0+, and when there's multiple embeds on a page, then it blocks loading for many seconds without showing any indication (spinner etc).
I'm not sure if this is caused by pure JS blockers, or if this is something in the connection handling (Jetty etc). I have tried to debug, but got lost. At least I have found when the problem started.

To Reproduce

  1. Create 8 questions, with embedding enabled. Something simple like SELECT 1 as "Q1", SELECT 2 as "Q2" etc
  2. Example embed loader (rename to .html) and update all the embed URLs
Show HTML code
<!DOCTYPE html>
<html>
<head>
    <title>Metabase Embedding Test</title>
</head>
<body>

<script type="text/javascript">
var startTime = Date.now();
var interval = setInterval(function() {
    var elapsedTime = Date.now() - startTime;
    document.getElementById("timer").innerHTML = (elapsedTime / 1000).toFixed(1);
}, 100);
window.onload = function() { clearInterval(interval); }
</script>
<h1 id="timer"></h1>

<!-- ->

<script type="text/javascript">
  window.onload = function(){
    for (let step = 0; step < 8; step++) {
        var f = document.createElement('iframe');
        f.src = "http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6Mjk2fSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ0ODMxLCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.V20fOWJ1Se9qeNd2lpORHuEMLREkJOE2Nx7fWMg10BY#bordered=true&titled=true";
        f.width = "400"; 
        f.height = "300";
        f.frameborder = "0";
        f.allowtransparency = "true";
        document.body.appendChild(f);
    }
  }
</script>

<!-- -->

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6Mjk2fSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ0ODMxLCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.V20fOWJ1Se9qeNd2lpORHuEMLREkJOE2Nx7fWMg10BY#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<!-- -->

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6Mjk3fSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ0ODg4LCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.XR744oNUn6sEiLmqGKKuNkLPlLF6OEgrxmAnPy1U6ZM#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6Mjk4fSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ0OTQzLCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.TPuKIsPht1cZ0Z5c_lD_KbYnSO573l9_kr3pcOr32cI#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6Mjk5fSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ0OTczLCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.SxQA96xEHOXIuIaNCjoYyaOJva80w95PkVmiXRrhL2I#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6MzAwfSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ1MDAzLCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.zcDkxlL_hPvSkI5uAzxovxKqa4QY2QrrscfR7eM64m0#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6MzAxfSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ1MDM2LCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.zawd6wwzv0SEzUHfrOtI1SjpDHdatLKUlZEOUBPV5Cs#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6MzAyfSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ1MDY0LCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.QD6onB-bYtrEPkIqpPVnnCQWR9JgqpMMJH1zFX_nYDQ#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<iframe src="http://localhost:3001/embed/question/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyZXNvdXJjZSI6eyJxdWVzdGlvbiI6MzAzfSwicGFyYW1zIjp7fSwiaWF0IjoxNjAxNjQ1MTMwLCJfZW1iZWRkaW5nX3BhcmFtcyI6e319.SdZYxiDyHjXk0gRbo9Q6q_79gAFaddEYUoTdRB3WZD0#bordered=true&titled=true"
        style="border:5px solid red;" onload="this.style.border='5px solid green';"
        width="400"
        height="300"
        allowtransparency></iframe>

<!-- -->

</body>
</html>
  1. Run the test on 0.35.0 (or any version above, I have tested up to 0.37.0-rc2)
    Screencast-2020-10-14_17 46 24
    • 1 embed will load in ~2.5 seconds, with visual spinner maybe showing .1 seconds before load.
    • 8 embeds will load in ~19 seconds, with no indication of what is happening until about 1 second before the end.
  2. Run the test on 0.34.3
    Screencast-2020-10-14_17 44 03
    • 1 embed will load in ~0.9 seconds, with visual spinner maybe showing .1 seconds before load.
    • 8 embeds will load in ~5 seconds, with visual spinner showing on all embeds around 4 second mark.

Another way to "reproduce":
Simply go to the embed preview in 0.34.3, which will load the preview much faster than any newer versions.

Expected behavior
At least non-blocking behavior, so a spinner is shown. Preferably sub-1-second load time.

@flamber flamber added Type:Bug Product defects Priority:P2 Average run of the mill bug Embedding/Static Static embedding, previously known as signed embedding .Performance labels Oct 14, 2020
@salsakran
Copy link
Contributor

Since this is both a regression, and a brutal slowdown (19s for 8 cards is not ok) I'm bumping this to a P1

@salsakran salsakran added Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness and removed Priority:P2 Average run of the mill bug labels Nov 3, 2020
@rlotun rlotun added this to the 0.37.3 milestone Nov 16, 2020
@rlotun rlotun modified the milestones: 0.37.3, 0.37.4 Dec 7, 2020
@ariya
Copy link
Contributor

ariya commented Dec 15, 2020

Thank you @flamber for such a detailed analysis!

I followed up your track and bisected all the ~200 commits between v0.34 and v0.35. Finally, I managed to narrow it down to single commit, 09001d6, that started to cause the embedding speed regression.

Looking at the front-end code difference between v0.34 and commit 09001d6 (and effectively, v0.35 or later), the JavaScript grows by approximately 280KB. Here is the breakdown:

In v0.34:

 3576195 resources/frontend_client/app/dist/vendor.bundle.js
    2487 resources/frontend_client/app/dist/styles.bundle.js
 1345125 resources/frontend_client/app/dist/app-embed.bundle.js
   13084 resources/frontend_client/app/dist/0.bundle.js
 4936891 TOTAL (4.71 MB)

In v0.35 (right after commit 09001d6):

 3763334 resources/frontend_client/app/dist/vendor.bundle.js
    2510 resources/frontend_client/app/dist/styles.bundle.js
 1460501 resources/frontend_client/app/dist/app-embed.bundle.js
 5226345 TOTAL (4.98 KB)

The above size difference causes the trigger to fetch the JSON (via XHR) to happen at different times as well. Notionly, it is a bit later for v0.35.

To demonstrate this, I captured the network waterfall diagram (see the attached HARs, can be displayed with e.g.Online HAR Viewer) for both v0.34 and v0.35. To further stress the front-end code, this was carried out under the "Fast 3G" network throttling simulation in Chrome Dev Tools.

v34.har.txt
v35.har.txt

With v0.34, the fetch is initiated after 8.69 seconds.

image

However, with v0.35, it happens almost a full second later, at 9.60 seconds.

image

This delay contributes to the slow down of iframe rendering for this embedding use case. Note that this is just testing a single (embedded) iframe. With a few more, there will be a cascading effect, adding further to the overall slow experience.

My next attemp is to figure out why this extra 280 KB of JavaScript code adds so much delay (~1 s) to the initial data fetch and if there is a way to workaround it.

@ariya
Copy link
Contributor

ariya commented Dec 16, 2020

For completeness, here is the code inflation due to the said commit, restricted to frontend/src:

FileBeforeAfterDelta
metabase-lib/lib/queries/StructuredQuery.js4291643211295
metabase-lib/lib/queries/structured/Aggregation.js68216813-8
metabase-lib/lib/queries/structured/Filter.js82958434139
metabase-lib/lib/queries/structured/MBQLClause.js2472252957
metabase/lib/expressions/compile.js054145414
metabase/lib/expressions/config.js56081817621
metabase/lib/expressions/format.js039273927
metabase/lib/expressions/formatter.js21550-2155
metabase/lib/expressions/index.js214847872639
metabase/lib/expressions/lexer.js078007800
metabase/lib/expressions/parser.js1630613146-3160
metabase/lib/expressions/process.js012911291
metabase/lib/expressions/suggest.js01551315513
metabase/lib/expressions/syntax.js01022410224
metabase/lib/expressions/tokens.js23080-2308
metabase/lib/query.js1363413631-3
metabase/lib/query/field_ref.js53105539229
metabase/lib/query/filter.js3080311737
metabase/lib/query_aggregation.js31903078-112
metabase/lib/schema_metadata.js1804818152104
metabase/modes/components/drill/UnderlyingRecordsDrill.jsx13332234901
metabase/query_builder/components/AggregationName.jsx3870392151
metabase/query_builder/components/AggregationPopover.jsx1387412115-1759
metabase/query_builder/components/DimensionList.jsx7687770720
metabase/query_builder/components/ExpressionPopover.jsx026012601
metabase/query_builder/components/ExtendedOptions.jsx66696611-58
metabase/query_builder/components/FieldList.jsx2919293415
metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx1147311241-232
metabase/query_builder/components/expressions/ExpressionWidget.jsx40464254208
metabase/query_builder/components/expressions/Expressions.jsx20791959-120
metabase/query_builder/components/expressions/TokenizedExpression.jsx3366703-2663
metabase/query_builder/components/expressions/TokenizedInput.jsx49265157231
metabase/query_builder/components/filters/FilterPopover.jsx557170981527
metabase/query_builder/components/filters/FilterPopoverHeader.jsx16081565-43
metabase/query_builder/components/filters/FilterPopoverPicker.jsx13121786474
TOTAL19797624667348697

The bulk of it is concentrated inside metabase/lib/expressions, therefore if we zoom in there:

FileBeforeAfterDelta
metabase/lib/expressions/compile.js054145414
metabase/lib/expressions/config.js56081817621
metabase/lib/expressions/format.js039273927
metabase/lib/expressions/formatter.js21550-2155
metabase/lib/expressions/index.js214847872639
metabase/lib/expressions/lexer.js078007800
metabase/lib/expressions/parser.js1630613146-3160
metabase/lib/expressions/process.js012911291
metabase/lib/expressions/suggest.js01551315513
metabase/lib/expressions/syntax.js01022410224
metabase/lib/expressions/tokens.js23080-2308
metabase/lib/query.js1363413631-3
metabase/lib/query/field_ref.js53105539229
metabase/lib/query/filter.js3080311737
metabase/lib/query_aggregation.js31903078-112
metabase/lib/schema_metadata.js1804818152104
TOTAL6673911380047061

My next step is find out why this extra 45 KB code adds so much to the initial delay of the (embedding) app.

@ariya ariya self-assigned this Dec 16, 2020
@ariya
Copy link
Contributor

ariya commented Dec 18, 2020

Another quick update.

I can now see an obvious place where the said commit introduces a new significant JavaScript code execution.

Before the commit, an illustrative example of the call tree from the code execution flame chart, before DOMContentLoaded event (which happens at 550 ms):

image

Chrome DevTools estimated the total blocking time to be ~167 ms.

In the meantime, after the commit was introduced (look at that deep, nested call tree!):

image

which contributed to a whopping (estimated) 730 ms blocking time. This also explains why DOMContentLoaded event fires after 1.12 seconds.

Next step: navigate the flame chart, understand the breakdown of the slow down even more.

@ariya
Copy link
Contributor

ariya commented Dec 18, 2020

I can now confirm that the slow-down is caused by Chevrotain's performSelfAnalysis, which needs to be called in the construction of the expression parsers (there are 3 instances of different parsers). In fact, this has nothing to do with embedding at all! The problem already manifests itself in the regular Metabase app, e.g. by going to the Sign In screen (however, the slow down is much much more perceptible when doing embedding, since there could be multiple iframes at once).

To reproduce:

  1. Launch Metabase, go to the Sign In screen
  2. Open Chrome DevTools, Performance panel
  3. Click Record, reload the page and once the page is completed, click Stop
  4. Drill-down the flame chart to locate the execution tree

As shown in the following screenshot, performSelfAnalysis is rather expensive (220 ms, and I'm on a fast 3.6 GHz desktop rig!) and it must be carried out for all the 3 different parsers. Because these steps happen in the beginning (application initialization), it delays the DOMContentLoaded up to 700 ms (or more, on a slower machine).

image

Next step: find a way to avoid this up-front expensive initialization.

@ariya
Copy link
Contributor

ariya commented Dec 24, 2020

This is now fixed with PR #14133.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Embedding/Static Static embedding, previously known as signed embedding .Performance Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness Type:Bug Product defects
Projects
None yet
Development

No branches or pull requests

4 participants