-
-
Notifications
You must be signed in to change notification settings - Fork 315
add sharding support to DiscordClient #1971
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
--------- Co-authored-by: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com>
Plerx2493
reviewed
Jun 30, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skimmed it
Plerx2493
requested changes
Jul 10, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some missing null checks but other than that RTM imo
Plerx2493
approved these changes
Jul 10, 2024
OoLunar
pushed a commit
that referenced
this pull request
Oct 3, 2024
* preliminary sharding design --------- Co-authored-by: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com> * we do need to disconnect too :P * implement a rudimentary transport service * reduce lifetime allocations of a DSharpPlus bot by up to one * support decompression in TransportService * commit to ask for feedback * slightly more state handling effort + options * alert GatewayClient of errors * able to start a single shard (in theory) * theoretically multi-shard orchestration works too now * delete DiscordClient.WebSocket.cs * make sharding clients possible to create * delete the legacy websocketclient * add support for getting latency and connection state * DiscordClient may receive events, as a treat * expose zombied to the user * remove the sharded client * implement sending payloads and reconnecting * remove more legacy ShardedClient code * make the shard count default to whatever Discord tells us * move reconnecting logic into GatewayClient * build * convenience methods * docs * xmldocs for IGatewayClient * works for single shards * don't send opcode 11 to dispatch * improve logging between shards * implement the 5s concurrency limit * accursed * don't explode on reconnecting * retry resuming if the connection cuts out * remove unused using * check whether all shards have connected before firing GDC --------- Co-authored-by: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com>
Instellate
pushed a commit
that referenced
this pull request
Nov 4, 2024
* preliminary sharding design --------- Co-authored-by: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com> * we do need to disconnect too :P * implement a rudimentary transport service * reduce lifetime allocations of a DSharpPlus bot by up to one * support decompression in TransportService * commit to ask for feedback * slightly more state handling effort + options * alert GatewayClient of errors * able to start a single shard (in theory) * theoretically multi-shard orchestration works too now * delete DiscordClient.WebSocket.cs * make sharding clients possible to create * delete the legacy websocketclient * add support for getting latency and connection state * DiscordClient may receive events, as a treat * expose zombied to the user * remove the sharded client * implement sending payloads and reconnecting * remove more legacy ShardedClient code * make the shard count default to whatever Discord tells us * move reconnecting logic into GatewayClient * build * convenience methods * docs * xmldocs for IGatewayClient * works for single shards * don't send opcode 11 to dispatch * improve logging between shards * implement the 5s concurrency limit * accursed * don't explode on reconnecting * retry resuming if the connection cuts out * remove unused using * check whether all shards have connected before firing GDC --------- Co-authored-by: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
big-change
core
docs
enhancement
epic-ioc
for pull requests and issues relating to IOC in v5
new-features
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #1908 , DiscordClient was migrated to being constructed through IoC, but DiscordShardedClient was left out.
This pull request brings sharding functionality to DiscordClient. Shards are now managed through an
IShardOrchestrator
, each shard is now represented by anIGatewayClient
. Reconnecting is managed according to the proposed changes in #1921. Shard orchestrators, gateway clients and the underlying transport services can be shimmed and/or overridden by the user through decorating or overwriting them in the service collection.Performance
This pull request rewrites most connection logic, retaining however Newtonsoft.Json for deserialization and thus retaining the requirement to convert all events into strings, nonetheless, the improved receive loop has brought substantial improvements:
(measurements taken on the following hardware: Ryzen 7950X, 64GB memory, 68GB pagefile; running Alpine Linux 3.20.1)
Please note that these benchmarks are conducted with fake data and are not necessarily representative of real-world performance, which may additionally be influenced by other factors present in real-world environments. The percentages are largely relative to other overhead (such as Newtonsoft.Json) and do not account for REST, which performed less than 10 requests for all three profiles, but which has also seen performance improvements in #1624
progresses towards #1820
closes #1921