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

Avoid expensive over-computing "all models" on Document init #11707

Closed
EfremBraun opened this issue Sep 30, 2021 · 9 comments · Fixed by #11739
Closed

Avoid expensive over-computing "all models" on Document init #11707

EfremBraun opened this issue Sep 30, 2021 · 9 comments · Fixed by #11739

Comments

@EfremBraun
Copy link
Contributor

EfremBraun commented Sep 30, 2021

Originally posted here: https://discourse.bokeh.org/t/poor-scaling-when-embedding-many-widgets-into-the-jinja-template/8456.

ALL software version info (bokeh, python, notebook, OS, browser, any other relevant packages)

Bokeh 2.4.0
Python 3.9.6
MacOs Catalina 10.15.7
Google Chrome 94.0.4606.61

Description of expected behavior and the observed behavior

I’ve been seeing poor runtime performance with my Bokeh application. After seeing https://discourse.bokeh.org/t/bokeh-version-0-12-10-incredibly-slow/2772, and noticing that my Console is giving me a lot of warnings about layout computes taking a long time, I decided to try using Bootstrap to layout the application, embedding individual Bokeh components into it.

This greatly improved the runtime performance. However, the loading time of the website is now quite abysmal. Running Chrome’s Lighthouse Audit (https://developer.chrome.com/docs/devtools/speed/get-started/), I see that the issue is the initial server response time. Basically, it’s taking a long time for the server to deliver the HTML page.

In debugging the issue, I’ve found that this embedding method works well when there are only a few Bokeh components, but it scales poorly as the number of Bokeh components increases, getting very slow once there are a few hundred components (and in my application, there are indeed a few hundred components).

I illustrate this issue here: https://github.com/EfremBraun/bokeh-scaling-demo. I took the Movies example from https://github.com/bokeh/bokeh/tree/branch-3.0/examples/app/movies and made some changes. original-0 was taken directly from the example, with 0 new widgets added, original-100 added 100 additional Slider widgets, and original-200 added 200 additional Slider widgets. embed-0 is the original example, but using embedding/Bootstrap for the display; embed-100 took that and added 100 additional Slider widgets, and embed-200 added 200 additional Slider widgets.

When I run bokeh serve original-0, open the application in a Chrome Incognito tab, and run the Lighthouse Audit, I get a 65 performance score. bokeh serve original-100 gets a 64, and bokeh serve original-200 also gets a 64. However, while bokeh serve embed-0 gets a 62 performance score (already lower than the original page, though not so bad), bokeh serve embed-100 gets a 58, and bokeh serve embed-200 gets a 54, and the page load time is noticeably slower. Screenshots showing these details can be seen below.

Screen Shot 2021-09-30 at 4 32 07 PM

Screen Shot 2021-09-30 at 4 32 09 PM

Screen Shot 2021-09-30 at 4 32 11 PM

Screen Shot 2021-09-30 at 4 32 12 PM

Screen Shot 2021-09-30 at 4 32 13 PM

Screen Shot 2021-09-30 at 4 32 27 PM

@mattpap
Copy link
Contributor

mattpap commented Sep 30, 2021

It's known issue that layout with many HTML widgets is slow (towards unusable). This will be fixed in bokeh 3.0.

@EfremBraun
Copy link
Contributor Author

EfremBraun commented Sep 30, 2021

I meant this issue to be about why the webpage is slow to load when many widgets are embedded in the document. I actually really like how I got my page to look using Bootstrap, so I'd be very happy to forego the Bokeh layouts entirely.

@EfremBraun
Copy link
Contributor Author

I believe I figured out an effective solution to this issue.

What slows down the bokeh server is the recompute() function call in https://github.com/bokeh/bokeh/blob/branch-3.0/bokeh/document/models.py. By wrapping the curdoc().add_root() calls within a with curdoc().models.freeze(): block, the recompute() function only gets called once at the very end. e.g., replace:

curdoc().add_root(desc)
curdoc().add_root(p)
for control in controls:
     curdoc().add_root(control)

with:

with curdoc().models.freeze():
    curdoc().add_root(desc)
    curdoc().add_root(p)
    for control in controls:
        curdoc().add_root(control)

I added embed-200-mod to the git repo above to demonstrate this. Running bokeh serve embed-200-mod and doing Chrome's Lighthouse Audit as before gets me a performance score of 57 (as opposed to bokeh serve embed-200 which gets a score of 54). This still sounds pretty bad, but the page loads far faster. I think that performance score doesn't take into account initial server response time.

Screen Shot 2021-10-01 at 12 21 40 PM

@EfremBraun
Copy link
Contributor Author

Perhaps it's worth documenting this technique in https://docs.bokeh.org/en/latest/docs/user_guide/embed.html#standard-template? I can put in a pull request for that if you guys agree.

@bryevdv
Copy link
Member

bryevdv commented Oct 1, 2021

Perhaps it's worth documenting this technique

I am generally 👎 on enshrining clunky WARs in the official docs. Also the freeze API you are using should never have been public in the first place, and quite possibly should be deprecated. I should warn that using freeze could easily lead to unexpected undesirable results. 100% YMMV. In any case I'm more inclined to leave any documentation updates for a proper solution.

@EfremBraun
Copy link
Contributor Author

Ah, I didn't think it would be an issue since I saw "Recompute the set of all models based on references reachable from the Document's current roots. This computation can be expensive. Use freeze to wrap operations that update the model object graph to avoid over-recompuation" in the docs for recompute() in https://github.com/bokeh/bokeh/blob/ca239b9a92605710f4ea6681da29696979d98824/bokeh/document/models.py.

@bryevdv
Copy link
Member

bryevdv commented Oct 3, 2021

@EfremBraun the problem is that not re-computing in unexpected ways or places may break things, or who knows what. I cannot give you any guarantees whatsoever about the current or future stability or workability of what you are doing above. Yes, the functions happen to have decent doctrings. But they are not mentioned anywhere in the user guide or tutorials or ever used in in any examples, and I have never directed anyone to them in support forums. Their being "public" is purely a historical accident from the very earliest days of the current Bokeh server. They are lowest-level dev API and not meant for users to use directly. As I said, 100& YMMV.

@EfremBraun
Copy link
Contributor Author

I looked into methods to further speed up the loading time of a Bokeh page that has many widgets embedded into the Jinja template.

Running bokeh serve embed-200-mod with BOKEH_LOG_LEVEL set to debug showed me that one thing slowing down the page loading was the frequent calls to _recompute_all_models() in https://github.com/bokeh/bokeh/blob/branch-3.0/bokehjs/src/lib/document/document.ts. Every call to add_root() calls _pop_all_models_freeze() which calls _recompute_all_models(). However, _recompute_all_models() only needs to be called once at the very end of all add_root() calls. Calling _recompute_all_models() multiple times on each add_root() just detaches and re-attaches the widgets again and again. See this in action here:

original.mov

By commenting out lines 200-202 of https://github.com/bokeh/bokeh/blob/branch-3.0/bokehjs/src/lib/document/document.ts and adding in doc._recompute_all_models() between lines 650 and 651, the expensive recomputation only occurs once at the end. See this in action here:

faster.mov

This change increases the Lighthouse Audit performance score of the application from 57 to 59, decreasing the load time by a not-so-noticeable 0.1 second. However, on my own application, it increases the Lighthouse Audit performance score of the application from 16 to 45, decreasing the load time by a VERY noticeable 1.1 seconds.

I don't really see any reason that this would give unexpected side effects, and I haven't seen anything strange in my own application. However, I'm new to web development, so I could be wrong. Do you think I should put in a pull request for this? I'll definitely be wanting to do this for my own application...

@bryevdv
Copy link
Member

bryevdv commented Oct 13, 2021

@EfremBraun Why don't you submit a PR with this change, and we can start by seeing how the full CI does.

@bryevdv bryevdv added this to the 3.0 milestone Oct 22, 2021
@bryevdv bryevdv changed the title [BUG] Poor scaling when embedding many widgets into the Jinja template Avoid expensive over-computing "all models" on Document init Oct 22, 2021
@bryevdv bryevdv modified the milestones: 3.0, 2.4.3 May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants