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

update_dependency_cache! performance bottleneck #2076

Closed
fonsp opened this issue Apr 29, 2022 · 6 comments · Fixed by #2080 or #2079
Closed

update_dependency_cache! performance bottleneck #2076

fonsp opened this issue Apr 29, 2022 · 6 comments · Fixed by #2080 or #2079
Labels
backend Concerning the julia server and runtime performance

Comments

@fonsp
Copy link
Owner

fonsp commented Apr 29, 2022

Here is a profile of changing the bond value of x (i.e. moving the Slider) in the PlutoUI sample notebook, which takes a surprisingly long time (90ms for me):

image

It shows that update_dependency_cache! is the bottleneck, taking about 2/3 of the total time. Since this is a non-essential feature, and because the topology does not change when running bonds, it should be possible to eliminate it.

bond value benchmark.jl.txt

@fonsp fonsp added backend Concerning the julia server and runtime performance labels Apr 29, 2022
@fonsp
Copy link
Owner Author

fonsp commented Apr 29, 2022

Maybe related, there are still lots of unresolved cells after running the notebook:

image

@Pangoraw

@fonsp
Copy link
Owner Author

fonsp commented Apr 29, 2022

#2077 fixed the unresolved_cells issue, which fixes the performance issue in sample/PlutoUI.jl.jl, because update_dependency_cache! is no longer begin called during set_bond_values_reactive! (here).

However, if any cells contains a macrocall that can not be resolved, then this issue becomes noticeable again. Notice that by removing @asdf (the only unresolved cell), performance increases again:

Schermopname.2022-04-29.om.16.28.51.mov

@Pangoraw
Copy link
Collaborator

Maybe we could use a != equality check here?

if cached === nothing || cached.input_topology !== notebook.topology

@fonsp
Copy link
Owner Author

fonsp commented Apr 29, 2022

@Pangoraw I also traced back the problem to this point! I made #2079, which would be equivalent to your suggestion, right?

@fonsp
Copy link
Owner Author

fonsp commented Apr 29, 2022

#2079 This PR took out the first ⛰ in the profile:

Schermafbeelding 2022-04-29 om 17 18 53

New profile

Schermafbeelding 2022-04-29 om 17 20 47

@fonsp
Copy link
Owner Author

fonsp commented Apr 29, 2022

Fixed! Finishing my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Concerning the julia server and runtime performance
Projects
None yet
2 participants