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

From 188c81b on some pages load extremely slowly #2926

Closed
Wramberg opened this issue Apr 21, 2024 · 9 comments
Closed

From 188c81b on some pages load extremely slowly #2926

Wramberg opened this issue Apr 21, 2024 · 9 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Wramberg
Copy link

Description

I have a page with many expandable fields made like this

expand_button = ui.button(icon="expand_more").props("flat size=10px dense")
with ui.column().style("gap: 5px; padding-left: 20px;") as details:
    for children_issue in sort_issue_list(issue.epic_children()):
        render_issue(children_issue, jira_data)

def toggle_visibility(element):
    element.set_visibility(not element.visible)

toggle_visibility(details)
expand_button.on("click", lambda: toggle_visibility(details))

I have binary searched my way from 1.4.5 to latest version and my issue is introduced with ebdf467

When I open my page it takes ~15 seconds to become responsive. Before the commit it takes ~500 ms.

@Wramberg Wramberg changed the title After ebdf46787e817614cf7d908d454f312f439b6f85 my page loads extremely slow and firefox warns that the page is slowing down the browser After ebdf467 my page loads extremely slow and firefox warns that the page is slowing down the browser Apr 21, 2024
@falkoschindler
Copy link
Contributor

Thanks for reporting this issue, @Wramberg!
That sounds very strange, because ebdf467 only affects ui.expansion, which doesn't occur in your example. But maybe it's missing some crucial part.
Can you, please, try to provide a self-containing minimal reproducible example? Thanks!

@falkoschindler falkoschindler added the question Further information is requested label Apr 22, 2024
@Wramberg
Copy link
Author

Wramberg commented Apr 22, 2024

Seems like its related to simply the amount of elements on the page. Consider this complete example

#!/usr/bin/env python3
from nicegui import ui

DRAWER_STYLE = "background-color: #3d3d3f; padding: 0"


def main():
    ui.run(
        title="PBU.DEV.SW Overview",
        port=8083,
        show=False
    )


@ui.page("/")
def root_page():
    add_global_settings()
    tabs = render_left_drawer()

    with ui.tab_panels(tabs, value="Release Plan").classes("w-full").props(
        "transition-next=jump-up transition-prev=jump-down"
    ):
        with ui.tab_panel("Release Plan"):
            render_page()



def add_global_settings():
    ui.colors(primary="#3d3d3f", secondary="#303032 ", accent="#a2bd30")
    ui.query("body").style("font-family: 'Source Sans Pro', sans-serif;")
    ui.add_head_html(
        """
    <style>
    .q-tabs--vertical .q-tab__indicator {
        width: 3px;
    }
    </style>
    """
    )


def render_left_drawer():
    with ui.left_drawer(top_corner=True, bottom_corner=True).style(DRAWER_STYLE).props("dark width=200 breakpoint=0"):
        with ui.tabs().props(
            "indicator-color=accent active-bg-color=none active-color=accent vertical inline-label"
        ).classes("w-full") as tabs:
            ui.tab("Release Plan").style("background-color: #303032")

    return tabs


def render_page():
    for i in range(300):
        render_issue()


def render_issue():
    with ui.element("div") as div:
        render_issue_icon()
        render_issue_link()

    return div


def render_issue_icon():
    ui.icon("assignment", color="purple")


def render_issue_link():
    issue_text = f"[testmwe] Some more text goes here just for testing"
    issue_url = f"google.com"
    ui.link(issue_text, issue_url).style(
        "padding-left: 5px; display: inline; color: inherit; text-decoration: inherit"
    )


if __name__ in ["__mp_main__", "__main__"]:
    main()

With the following docker file I get almost instant page load and I can instantly click links etc.

FROM ubuntu:22.04

RUN apt-get update && \
    DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
    git \
    python3 \
    python3-pip \
    && rm -rf /var/lib/apt/lists/*

RUN python3 -m pip install --default-timeout=100 numpy matplotlib scipy fastapi==0.108.0 jira
RUN python3 -m pip install git+https://github.com/zauberzeug/nicegui.git@d641cdfdac51c9a89edb710813eaeb5807e516cd

With this docker file I also get instant page load but it hangs for over 10 seconds before I can actually click links etc. Like all the javascript logic is hanging.

FROM ubuntu:22.04

RUN apt-get update && \
    DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
    git \
    python3 \
    python3-pip \
    && rm -rf /var/lib/apt/lists/*

RUN python3 -m pip install --default-timeout=100 numpy matplotlib scipy fastapi==0.108.0 jira
RUN python3 -m pip install git+https://github.com/zauberzeug/nicegui.git@ebdf46787e817614cf7d908d454f312f439b6f85

I run it like so

docker run -d --mount type=bind,source="$(pwd)",target=/workdir/ -p 8082:8083 IMAGENAME /workdir/run_web_ui.py

@falkoschindler
Copy link
Contributor

falkoschindler commented Apr 23, 2024

I think I start to understand:

The commit ebdf467 your blamed is just a merge commit from main to "outbox-delay". The previous commit d641cdf was on the main branch, while ebdf467 is on "outbox-delay". So it's not really the fault of ebdf467, but some earlier commit on the "outbox-delay" branch.

Screenshot 2024-04-23 at 15 13 22

I think we need to compare d31782e (before merging "outbox-delay") and 95408bd (after merging "outbox-delay").

  1. I reduced your code to the following test.py:

    #!/usr/bin/env python3
    from nicegui import ui
    
    @ui.page("/")
    def root_page():
        ui.colors(primary="#3d3d3f", secondary="#303032", accent="#a2bd30")
        ui.query("body").style("font-family: 'Source Sans Pro', sans-serif;")
        ui.add_head_html('<style>.q-tabs--vertical .q-tab__indicator { width: 3px; }</style>')
        with ui.left_drawer(top_corner=True, bottom_corner=True).style("background-color: #3d3d3f; padding: 0").props("dark width=200 breakpoint=0"):
            with ui.tabs().props("indicator-color=accent active-bg-color=none active-color=accent vertical inline-label").classes("w-full") as tabs:
                ui.tab("Release Plan").style("background-color: #303032")
        with ui.tab_panels(tabs, value="Release Plan").classes("w-full").props("transition-next=jump-up transition-prev=jump-down"):
            with ui.tab_panel("Release Plan"):
                for _ in range(300):
                    with ui.element():
                        ui.icon("assignment", color="purple")
                        ui.link("[testmwe] Some more text goes here just for testing", "google.com").style(
                            "padding-left: 5px; display: inline; color: inherit; text-decoration: inherit")
    
    ui.run(port=8000)
  2. I created Dockerfile1 and Dockerfile2 from the two commit hashes:

    FROM ubuntu:22.04
    
    RUN apt-get update && \
        DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
        git \
        python3 \
        python3-pip \
        && rm -rf /var/lib/apt/lists/*
    
    RUN python3 -m pip install --default-timeout=100 numpy matplotlib scipy fastapi==0.108.0 jira
    RUN python3 -m pip install git+https://github.com/zauberzeug/nicegui.git@d31782e5052724a0c68cf254e25b4ce047db87f0
    FROM ubuntu:22.04
    
    RUN apt-get update && \
        DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
        git \
        python3 \
        python3-pip \
        && rm -rf /var/lib/apt/lists/*
    
    RUN python3 -m pip install --default-timeout=100 numpy matplotlib scipy fastapi==0.108.0 jira
    RUN python3 -m pip install git+https://github.com/zauberzeug/nicegui.git@95408bd335d8f6a77406267c2324648b6bf1d317
  3. I built and run both images like this:

    docker build -f Dockerfile1 -t image1 .
    docker build -f Dockerfile2 -t image2 .
    
    docker run -d --mount type=bind,source="$(pwd)",target=/workdir/ -p 8001:8000 image1 /workdir/test.py
    docker run -d --mount type=bind,source="$(pwd)",target=/workdir/ -p 8002:8000 image2 /workdir/test.py

Interestingly both, http://localhost:8001 and http://localhost:8002, take a few seconds until the UI is responsive and the tab changes to bright green color - with Firefox as well as with Chrome. So it seems like the problem was introduced even earlier and we need to continue bisecting.

@falkoschindler falkoschindler added bug Something isn't working help wanted Extra attention is needed and removed question Further information is requested labels Apr 23, 2024
@falkoschindler falkoschindler changed the title After ebdf467 my page loads extremely slow and firefox warns that the page is slowing down the browser From ebdf467 on some pages load extremely slowly Apr 23, 2024
@Wramberg
Copy link
Author

That sounds odd. I will try to find time to re-test. But I am happy we at least agree that something looks odd. I was originally developing using nicegui version 1.4.5. It wasnt until I tried to deploy on another machine with more recent version i discovered the issue. So I would think the issued was introduced after 1.4.5 - but maybe verify my claims to make sure we are aligned :-)

@falkoschindler
Copy link
Contributor

I added a placeholder "HASH" to the Dockerfile

FROM ubuntu:22.04

RUN apt-get update && \
    DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
    git \
    python3 \
    python3-pip \
    && rm -rf /var/lib/apt/lists/*

RUN python3 -m pip install --default-timeout=100 numpy matplotlib scipy fastapi==0.108.0 jira
RUN python3 -m pip install git+https://github.com/zauberzeug/nicegui.git@HASH

and wrote this script to quickly test Git commit hashes:

#!/usr/bin/env python3
from pathlib import Path
import subprocess
import argparse

parser = argparse.ArgumentParser()
parser.add_argument('hash')
args = parser.parse_args()

Path('Dockerfile1').write_text(Path('Dockerfile').read_text('utf8').replace('HASH', args.hash), 'utf8')

containers = subprocess.run(['docker', 'ps', '-q'], stdout=subprocess.PIPE, text=True, check=False).stdout.split()
subprocess.run(['docker', 'stop', *containers], check=False)
subprocess.run(['docker', 'build', '-t', args.hash, '-f', 'Dockerfile1', '.'], check=True)
subprocess.run(['docker', 'run', '-d', '--mount', 'type=bind,source=.,target=/workdir/', '-p', '8001:8000', args.hash, '/workdir/test.py'], check=True)

This way I managed to bisect the issue to commit 188c81b, where the short freeze occurs for the first time. Its parent 8d1318a is still ok.

@falkoschindler falkoschindler changed the title From ebdf467 on some pages load extremely slowly From 188c81b on some pages load extremely slowly Apr 26, 2024
@falkoschindler
Copy link
Contributor

I think we need to talk about PR #2272. It basically started keeping updates and messages in the client's outbox until a connection is established. This was meant to solve problems like

@ui.page('/')
def page():
    label = ui.label('Hello world!')

    async def update():
        label.text = 'Hello again!'  # problem 1
        with label:
            ui.run_javascript('alert("Hello from JavaScript!")')  # problem 2
    background_tasks.create(update())

where some UI or JavaScript commands are created after the initial HTML has already been sent to the client but before the websocket is connected. But it causes all UI elements to be transmitted again, which is about 143KB for the test case from #2926 (comment).

By dropping such updates and messages the page loads much more quickly again. Here is an example for a rather drastic change in outbox.py:

                if not self.client.has_socket_connection:
                    await asyncio.sleep(0.1)
                    self.updates.clear()  # here
                    self.messages.clear()  # and here
                    continue

But we should re-think this feature once again. Which items need to be kept in the outbox? Can we somehow drop everything which is sent by the initial HTTP request? Could the client clear the outbox when sending the HTTP response?

@falkoschindler
Copy link
Contributor

falkoschindler commented Apr 26, 2024

Yes, this looks very promising:

    def build_response(self, request: Request, status_code: int = 200) -> Response:
        """Build a FastAPI response for the client."""
        self.outbox.updates.clear()
        self.outbox.messages.clear()
        ...

@rodja Let's discuss this change on Monday.


Update: There are two failing tests:

tests/test_notification.py ...E
tests/test_refreshable.py .....F..

@falkoschindler falkoschindler added this to the 1.4.24 milestone Apr 26, 2024
@falkoschindler falkoschindler removed the help wanted Extra attention is needed label Apr 26, 2024
@falkoschindler falkoschindler self-assigned this Apr 26, 2024
@falkoschindler
Copy link
Contributor

Ah, clearing all messages is a bad idea, because they are not part of the initial HTML. But calling

self.outbox.updates.clear()

when building the HTTP response passes all tests and solves the issue of slowly loading pages.

@Wramberg
Copy link
Author

Wramberg commented May 4, 2024

@falkoschindler I see you managed to replicate, fix, etc. For the sake of closure I agree that my issue is fixed with 64eb3ae 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants