Skip to content

perf: Minor speedup to consenus metrics MarkLateVote (backport #3017)#3025

Merged
melekes merged 4 commits intov0.38.xfrom
mergify/bp/v0.38.x/pr-3017
May 7, 2024
Merged

perf: Minor speedup to consenus metrics MarkLateVote (backport #3017)#3025
melekes merged 4 commits intov0.38.xfrom
mergify/bp/v0.38.x/pr-3017

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented May 7, 2024

Minor speedup to metrics MarkLateVote.

I saw the excess string allocation calls, so made a quick PR to remove it. This saves between .18s-25s from the consensus mutex across this one hour block sync. (Likely not at all consensus critical) It also appears in receiving votes

image

(Saves ToLower(), .String(), .TrimPrefix() and newObject calls. The new call has comparable complexity to .String())


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
  • Title follows the Conventional Commits spec

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

Minor speedup to metrics MarkLateVote.

I saw the excess string allocation calls, so made a quick PR to remove
it. This saves between .18s-25s from the consensus mutex across this one
hour block sync. (Likely not at all consensus critical) It also appears
in receiving votes

![image](https://github.com/cometbft/cometbft/assets/6440154/61875b0d-b419-4900-a74c-76c65087bca4)

(Saves ToLower(), .String(), .TrimPrefix() and newObject calls. The new
call has comparable complexity to .String())

---

#### PR checklist

- [x] Tests written/updated
- [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 4a37c00)

# Conflicts:
#	consensus/metrics.go
@mergify mergify bot requested a review from a team as a code owner May 7, 2024 06:47
@mergify
Copy link
Contributor Author

mergify bot commented May 7, 2024

Cherry-pick of 4a37c00 has failed:

On branch mergify/bp/v0.38.x/pr-3017
Your branch is up to date with 'origin/v0.38.x'.

You are currently cherry-picking commit 4a37c00da.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .changelog/unreleased/improvements/3017-speedup-consensus-metrics.md
	modified:   types/signed_msg_type.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   consensus/metrics.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot added the conflicts label May 7, 2024
@melekes melekes merged commit ed61568 into v0.38.x May 7, 2024
@melekes melekes deleted the mergify/bp/v0.38.x/pr-3017 branch May 7, 2024 07:34
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.

3 participants