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

Avoid HashMap in ShardQueue #2750

Merged
merged 1 commit into from
Feb 11, 2024
Merged

Conversation

GnomedDev
Copy link
Member

@GnomedDev GnomedDev commented Jan 31, 2024

This avoids hashing costs, is probably more cache efficient, and avoids storing duplicate information (length of buckets and max concurrency are equal).

@github-actions github-actions bot added the gateway Related to the `gateway` module. label Jan 31, 2024
@mkrasnitski
Copy link
Collaborator

Actually, we could even use a regular Vec to avoid pulling in extra machinery. Under the hood, VecMap uses a Vec<Option<V>>, but storing VecDeque::new() is pretty much the same semantically as storing None, so just making it a Vec<VecDeque<ShardId>> would work.

@GnomedDev
Copy link
Member Author

Sure, good idea.

@GnomedDev GnomedDev changed the title Use VecMap for ShardQueue Avoid HashMap in ShardQueue Jan 31, 2024
src/gateway/bridge/shard_queuer.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot added http Related to the `http` module. examples Related to Serenity's examples. labels Jan 31, 2024
@github-actions github-actions bot added the builder Related to the `builder` module. label Jan 31, 2024
@mkrasnitski
Copy link
Collaborator

Looking at your changes in small-fixed-array 0.3, it seems the type inference for indices is a problem. I don't think it makes sense to provide more than one implementation of Index. Sticking with usize is probably best for now.

@GnomedDev
Copy link
Member Author

I definitely see that, but I believe that the type inference problems are worth it to make indexing using the length type provided work.

@mkrasnitski
Copy link
Collaborator

I definitely see that, but I believe that the type inference problems are worth it to make indexing using the length type provided work.

I don't quite understand the benefit of indexing with the length type. What does it provide?

@GnomedDev
Copy link
Member Author

If the type for indexing does not match the type returned from len, a lot of basic patterns become a pain. Further discussion on this should be done on the small-fixed-array repo, as this is not the right place for it.

@GnomedDev GnomedDev marked this pull request as draft February 6, 2024 10:15
@github-actions github-actions bot added model Related to the `model` module. cache Related to the `cache`-feature. utils Related to the `utils` module. and removed builder Related to the `builder` module. examples Related to Serenity's examples. labels Feb 6, 2024
@github-actions github-actions bot removed model Related to the `model` module. cache Related to the `cache`-feature. http Related to the `http` module. utils Related to the `utils` module. labels Feb 11, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Mar 13, 2024
GnomedDev added a commit that referenced this pull request Mar 13, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Mar 19, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Mar 19, 2024
GnomedDev added a commit that referenced this pull request Mar 21, 2024
GnomedDev added a commit that referenced this pull request Mar 25, 2024
GnomedDev added a commit that referenced this pull request Mar 25, 2024
GnomedDev added a commit that referenced this pull request Mar 29, 2024
GnomedDev added a commit that referenced this pull request Mar 31, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Mar 31, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Apr 1, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request May 14, 2024
GnomedDev added a commit that referenced this pull request May 14, 2024
GnomedDev added a commit that referenced this pull request May 23, 2024
GnomedDev added a commit that referenced this pull request May 28, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Jun 9, 2024
GnomedDev added a commit that referenced this pull request Jun 22, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Jun 22, 2024
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jul 29, 2024
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jul 30, 2024
GnomedDev added a commit that referenced this pull request Aug 16, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Oct 7, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Oct 20, 2024
GnomedDev added a commit that referenced this pull request Oct 20, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Nov 11, 2024
GnomedDev added a commit that referenced this pull request Nov 13, 2024
GnomedDev added a commit to GnomedDev/serenity that referenced this pull request Nov 15, 2024
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Dec 8, 2024
arqunis pushed a commit to arqunis/serenity that referenced this pull request Jan 16, 2025
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement to Serenity. gateway Related to the `gateway` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants