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

xdr and historyarchive: Remove XDR decoder allocations from XdrStream #4075

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

2opremio
Copy link
Contributor

Also:

  • Move Uint32Ptr into test file (it was only used for testing)
  • Tidy up EncoderTo and DecoderFrom interfaces

Also:
* Move Uint32Ptr into test file (it was only used for testing)
* Tidy up EncoderTo and DecoderFrom interfaces
@2opremio
Copy link
Contributor Author

@bartekn , @tamirms Please let me know if there are other places in Horizon which would benefit from a similar pattern.

@tamirms
Copy link
Contributor

tamirms commented Nov 16, 2021

Please let me know if there are other places in Horizon which would benefit from a similar pattern

I found 3 more places which I think would benefit from using decoders / encoders:

(1) https://github.com/stellar/go/blob/master/ingest/ledgerbackend/buffered_meta_pipe_reader.go#L96
This is where we read ledgers from the captive core pipe

(2) https://github.com/stellar/go/blob/master/ingest/ledgerbackend/remote_captive_core.go#L115
This is where we read ledgers from the http response that we receive from the remote captive core server

(3) https://github.com/stellar/go/blob/master/exp/services/captivecore/internal/api.go#L184
This is where we serialize the ledger into base 64 before in the captive core server

@2opremio 2opremio merged commit 8981dfd into stellar:master Nov 16, 2021
@2opremio
Copy link
Contributor Author

Great, I will take a look

@2opremio 2opremio deleted the save-xdrstream-decoder-allocs branch November 16, 2021 14:33
@2opremio
Copy link
Contributor Author

I optimized the captive core pipe decoding at #4077

About remote captive core, I saw there was json marshaling/unmarshaling involved which is done through reflection.

I don't think that an extra XDR decoder allocation is going to make a big difference before optimizing the json encoding. I may be wrong though so @tamirms feel free to take a look at it yourself (now I am curious).

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