perf: enhance performance in state #1977
Conversation
|
Needs #1954 |
|
@ValarDragon - I don't think that this is quite done correctly yet, but could you take a peek? |
|
oh I def found a bug.... it is strange that tests pass |
|
I was afraid of that... Note: need to undo some doings and redo some others. |
This reverts commit d96822e.
|
@ValarDragon @melekes - what would be a good way to benchmark here? And what should be "the standard"? |
|
This isn't the same thing reported by @ValarDragon but I think it's related: |
| return dbStore{db, options} | ||
| return &dbStore{ | ||
| db: db, | ||
| valInfoCache: make(map[int64]*cmtstate.ValidatorsInfo), |
There was a problem hiding this comment.
Why do we need these? I don't see them used anywhere in this PR
There was a problem hiding this comment.
I believe that I made a branch management mistake here. Thanks for review.
There was a problem hiding this comment.
Hey, I'm wrong. Basically one finding was that it was faster to use pointers and lay things out first. That's why that's there.
Could you let me know if that is a decent direction?
I prefer the original form of the code, and the current performance. It was hard to determine exactly where perf improvements came from though.
57aca9b to
9260202
Compare
|
@melekes I will do more here, once has been merged. |
|
I'm opening this for review again -- close if anyone thinks is not needed, it seems like I went outside of the original intended scope. |
|
Hey I'm going to close this with the note to self that we should have more benchmarks across the code base |
Working on #1976 and also examining other ways to improve performance.
closes: #1976
PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments