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

allow RepaintSignal: ?Sync #1187

Closed
wants to merge 10 commits into from
Closed

Conversation

zetanumbers
Copy link

Closes #1186.

@zetanumbers zetanumbers marked this pull request as ready for review January 31, 2022 22:04
@emilk
Copy link
Owner

emilk commented Feb 2, 2022

Is all this new code just to avoid the performance impact of locking a mutex when the users requests a repaint? Or is there some other benefit I'm not seeing?

@zetanumbers
Copy link
Author

Is all this new code just to avoid the performance impact of locking a mutex when the users requests a repaint? Or is there some other benefit I'm not seeing?

This PR firstly fixes logic of the library and only then the performance. There are several of benefits to it:

  • Avoids mutex poisoning and panic deadlocks (i. e. if some thread panics while using request_repaint, then other threads would panic on a request_repaint too);
  • Use of DST eliminates an indirection;
  • Avoids unnecessary locks (majority of channel Senders (like mpsc) are Send + !Sync which are used by window handling libraries);
  • There may be other downsides of relying on mutex, like if it's not implemented for some platform or another indirection (posix targets store mutex on the heap IIRC).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RepaintSignal: Sync
3 participants