Skip to content

perf(libs/json): Lower heap overhead of JSON encoding (backport #2846)#2874

Merged
andynog merged 1 commit intov1.xfrom
mergify/bp/v1.x/pr-2846
Apr 23, 2024
Merged

perf(libs/json): Lower heap overhead of JSON encoding (backport #2846)#2874
andynog merged 1 commit intov1.xfrom
mergify/bp/v1.x/pr-2846

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Apr 23, 2024


Many RPC methods require JSON marshalled responses. We saw this taking a notable amount of heap allocation in query serving full nodes. This PR removes some extra heap allocations that were being done. We avoided using the more efficient encoder.Encode before, because it added a newline. This PR changes the function signature for these private methods to be using *bytes.Buffer, and then uses the in-buffer methods (rather than a second copy). We then just truncate the final byte after each such call, which does not waste any allocations.

I added a benchmark for the most complex test case.

OLD:

BenchmarkJsonMarshalStruct-12              78992             15542 ns/op            4487 B/op        191 allocs/op

New:

BenchmarkJsonMarshalStruct-12              93346             11132 ns/op            3245 B/op         58 allocs/op

Roughly a 3-4x reduction in the number of allocations, and 20% speedup.

PR checklist

  • Tests written/updated - Existing tests cover this
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

This is an automatic backport of pull request #2846 done by [Mergify](https://mergify.com).

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

Many RPC methods require JSON marshalled responses. We saw this taking a
notable amount of heap allocation in query serving full nodes. This PR
removes some extra heap allocations that were being done. We avoided
using the more efficient encoder.Encode before, because it added a
newline. This PR changes the function signature for these private
methods to be using *bytes.Buffer, and then uses the in-buffer methods
(rather than a second copy). We then just truncate the final byte after
each such call, which does not waste any allocations.

I added a benchmark for the most complex test case.

OLD:
```
BenchmarkJsonMarshalStruct-12              78992             15542 ns/op            4487 B/op        191 allocs/op
```
New:
```
BenchmarkJsonMarshalStruct-12              93346             11132 ns/op            3245 B/op         58 allocs/op
```

Roughly a 3-4x reduction in the number of allocations, and 20% speedup.

#### PR checklist

- [x] Tests written/updated - Existing tests cover this
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit eb09601)
@mergify mergify bot requested a review from a team as a code owner April 23, 2024 12:49
@mergify mergify bot requested a review from a team April 23, 2024 12:49
@andynog andynog self-assigned this Apr 23, 2024
Copy link
Collaborator

@andynog andynog left a comment

Choose a reason for hiding this comment

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

lgtm

@andynog andynog merged commit 70ce71f into v1.x Apr 23, 2024
@andynog andynog deleted the mergify/bp/v1.x/pr-2846 branch April 23, 2024 13:33
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