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

~50X performance regression to Vaadin 8 when rendering buttons in vertical layout #5447

Closed
mstahv opened this issue Apr 9, 2019 · 6 comments
Milestone

Comments

@mstahv
Copy link
Member

mstahv commented Apr 9, 2019

There seems to be a serious performance regression whend rendering simple components. Following code snippet takes over 15 seconds to to appear in browser on my local test setup. In Vaadin 8 a similar UI renders in a snap, with debug mode on: "Processing time was 246ms
". Neither JVM or browser uses excessive CPU. Heap with one page render: ~ 30 in V8, 36 in V10, didn't check actual session size.

During the load, there is only blank white screen shown (except in FF where browser says that a lot of energy is used, shall I stop execution)

    for (int i = 0; i < 1000; i++) {
        int finalI = i;
        add(new Button("button " + i, e -> Notification.show("Click from " + finalI)));
    }

Update: Tested that V13 is around 15 seconds faster if one switches to use NativeButton ("aka element) instead of vaadin-button based Button. So this is most likely a vaadin-button issue, more than Flow issue.

@pleku
Copy link
Contributor

pleku commented Apr 10, 2019

First, the snippet could be full, so one could just copy it to get started with testing this (just have the @Route class MyView extends VerticalLayout {).

Also you're referring to both V10 and V13, so a bit confused which version it is.

Following code snippet takes over 15 seconds to to appear in browser on my local test setup.

V13 is around 15 seconds faster if one switches to use NativeButton ("aka element)

Does that mean that the time is 0-1 seconds then ?

Anyway, it might not just be about the vaadin-button, but rather in the way the Polymer 2 web components are rendered in general or with Flow. Rendering 1000 non-web component elements is faster than rendering P2 web components, that is an unfortunate fact.

So for a good comparison benchmark, one should test how much time it takes to render 1000 vaadin-buttons in a vaadin-vertical-layout without Flow.

Meanwhile, a workaround for this issue without switching to NativeButton is to use IronList component, it makes no sense to load 1000 buttons vertically because the user can see only a subset of those. I'm not sure about the Button styles, but it might be that you'll need to wrap it with a Div though to be able to apply spacing easily.

@Artur-
Copy link
Member

Artur- commented Apr 10, 2019

Commenting out the problematic _updateAriaLabel function in vaadin-button reduces rendering time from 13s to 800ms

@phauer
Copy link

phauer commented Apr 10, 2019

First of all, thank you all for looking at this issue. That's great!

I also believe that this is a general issue affecting all new Vaadin 10+ components using Polymer and Shadow DOM. I also had a view containing ~ 100 FormLayouts and recognized a poor rendering performance too. So I switch to DIv and added some styles. This fixed the performance issue.

I'm using Vaadin 13.

I'll try the NativeButton.

@pleku
Copy link
Contributor

pleku commented Apr 10, 2019

So for vaadin-button the issue is vaadin/vaadin-button#128 and we're already reverting:

Another good discovery by @Artur- that should be documented here too that using innerText is the source of all evil https://stackoverflow.com/questions/19256864/why-does-replacing-innerhtml-with-innertext-causes-15x-drop-in-performance

Well have to go through and try to replace any usage of innerText in the framework too.

@pleku
Copy link
Contributor

pleku commented Apr 10, 2019

For the framework and the web component Java integrations, I could not find any usages of innerText other than in the push.js file for the response handling.

I could however find lots of usages of innerText in the vaadin-grid web component, which will be looked after by the components team.

The form-layout issue is another thing; the responsive steps causing lots of redundant(?) recalculations. I'm pretty sure there will an issue in vaadin/vaadin-form-layout soon.

Closing this issue as duplicate of vaadin/vaadin-button#128. Please comment here or open another issue if it is still valid after that one is fixed.

@pleku pleku closed this as completed Apr 10, 2019
@pleku pleku added this to the Abandoned milestone Apr 10, 2019
@phauer
Copy link

phauer commented Apr 10, 2019

Thank you very much!

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