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

services/horizon: Use encoding buffer in state verifier #4069

Merged
merged 1 commit into from
Nov 13, 2021

Conversation

2opremio
Copy link
Contributor

Profiling shows there are a lot of allocations due to buffer resizing.

@2opremio 2opremio requested a review from a team November 13, 2021 08:02
Profiling shows there are a lot of allocations due to buffer resizing.
@2opremio 2opremio force-pushed the verifier-encoding-buffer branch from 9bbc28a to 93b2223 Compare November 13, 2021 09:03
Copy link
Contributor

@tamirms tamirms left a comment

Choose a reason for hiding this comment

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

I think the encoding buffer could also be used by the checkpoint change reader which calls MarshalBinaryCompress() in two places:

keyBytes, e := key.MarshalBinaryCompress()

keyBytes, e := key.MarshalBinaryCompress()

There is also the XdrStream struct in the historyarchive package. We use that to extract xdr ledger entries out of the history archive snapshots. There are lots of decoding operations happening in XdrStream:

readi, err := xdr.Unmarshal(&x.buf, in)

Would it be possible to use a decoding buffer there? I think that would speed up state verification and the buildState step in the ingestion state machine.

@2opremio
Copy link
Contributor Author

2opremio commented Nov 13, 2021 via email

@2opremio 2opremio merged commit 9b9ddf1 into master Nov 13, 2021
@2opremio 2opremio deleted the verifier-encoding-buffer branch November 13, 2021 16:14
@2opremio
Copy link
Contributor Author

I think the encoding buffer could also be used by the checkpoint change reader which calls MarshalBinaryCompress() in two places:

Done at #4071

There is also the XdrStream struct in the historyarchive package. We use that to extract xdr ledger entries out of the history archive snapshots. There are lots of decoding operations happening in XdrStream:

I don't think it's worth optimizing this until stellar/xdrgen#71 is merged

@2opremio
Copy link
Contributor Author

There is also the XdrStream struct in the historyarchive package. We use that to extract xdr ledger entries out of the history archive snapshots. There are lots of decoding operations happening in XdrStream:

Addressed at #4075

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.

2 participants