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

perf(types): Speedup valset.GetByAddress by noticing we do not need t… (backport #95) #96

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jun 3, 2024

…o copy the validator object (cometbft#3129)

Speedup valset.GetByAddress by noticing we do not need to copy the validator object in ~every single usage, as it operates on read-only instances.

This acts as a very small speedup to ValidateBlock, and calculating statistics for enter prevote. In the future we can change data structures for bigger impact.

If we think this needs more tests to feel comfortable, I think we should close this PR. Was a driveby change, and there are more impactful things we can do


  • Tests written/updated
  • 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


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

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

ValarDragon and others added 2 commits June 3, 2024 20:41
…o copy the validator object (cometbft#3129)

Speedup valset.GetByAddress by noticing we do not need to copy the
validator object in ~every single usage, as it operates on read-only
instances.

This acts as a very small speedup to ValidateBlock, and calculating
statistics for enter prevote. In the future we can change data
structures for bigger impact.

If we think this needs more tests to feel comfortable, I think we should
close this PR. Was a driveby change, and there are more impactful things
we can do

<!--

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

-->

---

- [ ] Tests written/updated
- [ ] 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

---------

Co-authored-by: Andy Nogueira <[email protected]>
(cherry picked from commit 2e0a6db)
(cherry picked from commit f7684c5)
@ValarDragon ValarDragon merged commit e6ab6d6 into osmo-v25/v0.37.4 Jun 3, 2024
13 of 16 checks passed
@mergify mergify bot deleted the mergify/bp/osmo-v25/v0.37.4/pr-95 branch June 3, 2024 21:27
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.

1 participant