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

perf: reduce tokio tasks #45

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

JakkuSakura
Copy link
Contributor

This is a secondary PR to further reduce tokio tasks

@JakkuSakura JakkuSakura changed the title feat: reduce tokio takss WIPfeat: reduce tokio takss WIP Mar 31, 2023
@JakkuSakura JakkuSakura marked this pull request as draft March 31, 2023 17:58
@JakkuSakura JakkuSakura changed the title WIPfeat: reduce tokio takss WIP WIPfeat: reduce tokio takss Mar 31, 2023
(cherry picked from commit 645bc76)
…better_error_handling

# Conflicts:
#	src/session.rs
@JakkuSakura JakkuSakura marked this pull request as ready for review March 31, 2023 18:49
@JakkuSakura JakkuSakura changed the title WIPfeat: reduce tokio takss feat: reduce tokio takss Mar 31, 2023
@gbaranski gbaranski changed the title feat: reduce tokio takss perf: reduce tokio tasks Apr 1, 2023
src/axum.rs Outdated Show resolved Hide resolved
src/client.rs Outdated Show resolved Hide resolved
src/server.rs Outdated Show resolved Hide resolved
src/server.rs Outdated Show resolved Hide resolved
src/server.rs Outdated Show resolved Hide resolved
src/session.rs Outdated Show resolved Hide resolved
Copy link
Owner

@gbaranski gbaranski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somehow all connections are being dropped after 10 seconds.
It looks like no pongs are being sent as response to pings.

Also, are you sure that removing RawMessage was a good idea? I believe it was a good abstraction between what's handled in client.rs/server.rs and what is handled in socket.rs, e.g Ping/Pong didn't exist on Message, as they were handled by the Socket itself.

@JakkuSakura
Copy link
Contributor Author

JakkuSakura commented Apr 1, 2023 via email

@gbaranski
Copy link
Owner

Have you considered interoperability with other ws clients and servers? I definite need it without requiring clients to use certain framework

Yep ezsockets is definitely interoperable with other WebSocket libraries.
Ping/Pong is standardised thing, and according to RFC every client/server should reply to Ping frame with Pong

@JakkuSakura
Copy link
Contributor Author

I mean not every implementation follows the standard. Even for ping pong everyone do it differently: no ping-pong required, ping-pong as messages, use ws ping-pong, use ws ping-pong with certain messages

Ping/Pong didn't exist on Message, as they were handled by the Socket itself.

Socket can't handle every kind of ping-pong. So merging RawMessage to Message gives more flexibility to keep behavior of previous implementation, both client side and server side

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.

None yet

2 participants