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

Remove EncodedChunksCacheEntry in favor of PartialEncodedChunk #2635

Closed
SkidanovAlex opened this issue May 12, 2020 · 1 comment
Closed

Remove EncodedChunksCacheEntry in favor of PartialEncodedChunk #2635

SkidanovAlex opened this issue May 12, 2020 · 1 comment
Assignees
Labels
A-chain Area: Chain, client & related

Comments

@SkidanovAlex
Copy link
Collaborator

EncodedChunksCacheEntry and PartialEncodedChunk have largely the same fields, but the former uses Hash Tables while the latter uses vectors of options. Since the key of the hash tables are part ordinal (which is bounded by 255) and shard IDs (which short-medium term are also bounded in low hundreds), vectors of options are actually acceptable to be used in place of hash tables, and thus EncodedChunksCacheEntry is redundant.

Also at least inside process_partial_encoded_chunk_request we create the cache entry from a partial encoded chunk to respond to the request, which is potentially expensive, and redundant if we can instead use a partial encoded chunk directly.

@SkidanovAlex SkidanovAlex added the A-chain Area: Chain, client & related label May 12, 2020
birchmd added a commit to birchmd/nearcore that referenced this issue May 15, 2020
…PartialEncodedChunkResponse without header (this was the only case where the header was not used previously) (near#2635)
birchmd added a commit to birchmd/nearcore that referenced this issue May 15, 2020
…PartialEncodedChunkResponse without header (this was the only case where the header was not used previously) (near#2635)
birchmd added a commit to birchmd/nearcore that referenced this issue May 15, 2020
…PartialEncodedChunkResponse without header (this was the only case where the header was not used previously) (near#2635)
@birchmd
Copy link
Contributor

birchmd commented May 18, 2020

The core concern this issue cares about is the performance of process_partial_encoded_chunk_request. This is taken care of in #2646 so I will close this now.

@birchmd birchmd closed this as completed May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chain Area: Chain, client & related
Projects
None yet
Development

No branches or pull requests

2 participants