Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

akiraveliara
Copy link
Member

@akiraveliara akiraveliara commented Jun 22, 2024

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 an IGatewayClient. 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)

  • for 4.3.0, the receive loop took about 28.12% of total CPU time spent in the library, and was capable of handling approximately 4,000 events per second; however, this would rapidly degrade if the user handled any events, since user event handlers used to block the gateway. Thus, a more realistic measurement was often in the triple digits.
  • for 5.0.0-nightly-02303, the receive loop took about 15.42% of total CPU time spent in the library, and was capable of handling approximately 7,200 events per second.
  • for add sharding support to DiscordClient #1971, the receive loop took about 0.34% of total CPU time spent in the library, and combined with an earlier asynchronous dispatch was capable of handling approximately 94,000 events per second.

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

@akiraveliara akiraveliara added enhancement core epic-ioc for pull requests and issues relating to IOC in v5 labels Jun 22, 2024
@akiraveliara akiraveliara added this to the v5.0 milestone Jun 22, 2024
@akiraveliara akiraveliara mentioned this pull request Jun 30, 2024
36 tasks
Copy link
Member

@Plerx2493 Plerx2493 left a comment

Choose a reason for hiding this comment

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

Skimmed it

DSharpPlus/Clients/DiscordClient.cs Outdated Show resolved Hide resolved
DSharpPlus/Clients/DiscordClientBuilder.cs Outdated Show resolved Hide resolved
@akiraveliara akiraveliara marked this pull request as ready for review July 2, 2024 12:04
Copy link
Member

@Plerx2493 Plerx2493 left a 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

DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Clients/DiscordClient.Dispatch.cs Show resolved Hide resolved
DSharpPlus/Entities/Channel/DiscordChannel.cs Show resolved Hide resolved
@Plerx2493 Plerx2493 merged commit 1f36385 into master Jul 10, 2024
@Plerx2493 Plerx2493 deleted the aki/sharding branch July 10, 2024 20:13
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rework reconnection logic

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.