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

Set up Manual Threading how-to guide is highly problematic #6724

Open
MarcSkovMadsen opened this issue Apr 14, 2024 · 2 comments · May be fixed by #6727
Open

Set up Manual Threading how-to guide is highly problematic #6724

MarcSkovMadsen opened this issue Apr 14, 2024 · 2 comments · May be fixed by #6727
Labels
type: bug Something isn't correct or isn't working type: docs Related to the Panel documentation and examples
Milestone

Comments

@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Apr 14, 2024

I've been looking at the Set up Manual Threading multiple times and I think it will cause more harm the good.

  • It lacks the import panel as pn text.
  • It does not explain how to stop the thread when the session is ended. Thus more and more threads will be created.
  • It does not explain how to stop the thread when the app is autoreloaded.
  • Even though there is some attempt at explaining why use of c = threading.Condition() is nescessary I'm not sure the explanation is True. My understanding the Condition enables another thread to wait until a condition. And this is not what we do here. See https://docs.python.org/3/library/threading.html#condition-objects.
  • I lack some comment about safety of updating Panel components from separate threads. Is that safe? Why?

Maybe use of session life cycle hooks are solution? https://panel.holoviz.org/how_to/callbacks/session.html.


Furthermore the guide only explains how to use manual thread in main app.py file that is not shared between sessions. I would also like to see an explanation of using a single shared thread in external module. As in #6723. I think here concerns about performance comes in. Should this thread be pushing updates to all sessions that .depends on the threads updates? Or should each individual session be running a periodic callback to check for updates? What is most safe and performant?


[x] Yes. I'm willing to contribute a PR updating the guide. But I need some guidance and more understanding to be able to do it.

@MarcSkovMadsen MarcSkovMadsen added type: bug Something isn't correct or isn't working type: docs Related to the Panel documentation and examples labels Apr 14, 2024
@MarcSkovMadsen MarcSkovMadsen linked a pull request Apr 14, 2024 that will close this issue
@MarcSkovMadsen MarcSkovMadsen linked a pull request Apr 14, 2024 that will close this issue
@philippjfr philippjfr added this to the v1.4.2 milestone Apr 15, 2024
@philippjfr
Copy link
Member

A shared thread that processes queued events sounds very complex, that's really what the nthreads config option is for.

It does not explain how to stop the thread when the session is ended. Thus more and more threads will be created.

Seems like an easy fix, just add a on_session_destroyed handler.

It does not explain how to stop the thread when the app is autoreloaded.

Same thing.

Even though there is some attempt at explaining why use of c = threading.Condition() is nescessary I'm not sure the explanation is True.

Yeah, I don't think it's being used correctly, a simple Lock that guards reads and writes of the queue seems sufficient.

I lack some comment about safety of updating Panel components from separate threads. Is that safe? Why?

It's generally safe but of course if you have concurrent manipulation of the same component from multiple threads the standard precautions apply.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Apr 18, 2024

Thx. I don't know enough about threads to understand the last too comments. Could you make those more explicit:

Yeah, I don't think it's being used correctly, a simple Lock that guards reads and writes of the queue seems sufficient.

What would that look like in code terms?

It's generally safe but of course if you have concurrent manipulation of the same component from multiple threads the standard precautions apply.

What do you mean by standard precautions? What are they? Where can they be found?

Furthermore the guide only explains how to use manual thread in main app.py file that is not shared between sessions. I would also like to see an explanation of using a single shared thread in external module. As in #6723. I think here concerns about performance comes in. Should this thread be pushing updates to all sessions that .depends on the threads updates? Or should each individual session be running a periodic callback to check for updates? What is most safe and performant?

How would you do this? I don't think nthreads config options would help here.

@philippjfr philippjfr modified the milestones: v1.4.2, v1.4.3 Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't correct or isn't working type: docs Related to the Panel documentation and examples
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants