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.

Sign up for GitHub

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

Jump to bottom

ssmemstorage: nearly 5% of allocation in YCSB come from this package #89918

Open
ajwerner opened this issue Oct 13, 2022 · 4 comments
Open

ssmemstorage: nearly 5% of allocation in YCSB come from this package #89918

ajwerner opened this issue Oct 13, 2022 · 4 comments
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Oct 13, 2022 •
edited by cockroach-jira-scripts
Loading

Describe the problem
profile (32).pb.gz

Screen Shot 2022-10-13 at 1 23 01 PM

Jira issue: CRDB-20500
@ajwerner ajwerner added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Oct 13, 2022
@kevin-v-ngo
Copy link

kevin-v-ngo commented Oct 17, 2022 •
edited
Loading

Moving to Bugs. From triage, this may be related to the following issue: #89495. @matthewtodd, do you mind confirming? If so, we can close this issue. If not, we'll need a follow-up investigation.

@matthewtodd matthewtodd self-assigned this Oct 18, 2022
@matthewtodd
Copy link
Contributor

Andrew's work tuning insights changed the shape of the graph, but there's still a lot of low-hanging sqlstats fruit here:

Screen Shot 2022-10-26 at 3 59 01 PM

I've looked through the callsites, and the work seems pretty straightforward. I'll start sending a bunch of little PRs to fix, prioritized by impact.

@maryliag maryliag mentioned this issue Nov 23, 2022
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Nov 23, 2022
@maryliag
util: improve allocation of combine functions
688e79e 
Part Of cockroachdb#89918

Previously, the functions `CombineUniqueString` and
`CombineUniqueInt64` were always allocating a new object.
This change makes improvements on both functions.

Release note: None
maryliag added a commit to maryliag/cockroach that referenced this issue Nov 23, 2022
@maryliag
util: improve allocation of combine functions
2205278 
Part Of cockroachdb#89918

Previously, the functions `CombineUniqueString` and
`CombineUniqueInt64` were always allocating a new object.
This change makes improvements on both functions.

Release note: None
craig bot pushed a commit that referenced this issue Nov 24, 2022
@maryliag
Merge #92396
39250f6 
92396: util: improve allocation of combine functions r=maryliag a=maryliag

Part Of #89918

Previously, the functions `CombineUniqueString` and `CombineUniqueInt64` were always allocating a new object. This change makes improvements on both functions.

Results after running YCSB for a 10 minutes on each implementation:
https://www.loom.com/share/0073101092ae4dffacb254c0363f2132

Before
<img width="1604" alt="Screen Shot 2022-11-23 at 6 17 20 PM" src="https://user-images.githubusercontent.com/1017486/203662183-fcf74b13-a986-45df-b49c-ada82ab7231a.png">

After
<img width="1616" alt="Screen Shot 2022-11-23 at 6 17 01 PM" src="https://user-images.githubusercontent.com/1017486/203662242-744d49a2-234d-47d4-87ee-a304d21c1534.png">


Release note: None

Co-authored-by: maryliag <[email protected]>
blathers-crl bot pushed a commit that referenced this issue Nov 24, 2022
@maryliag
util: improve allocation of combine functions
2d80aae 
Part Of #89918

Previously, the functions `CombineUniqueString` and
`CombineUniqueInt64` were always allocating a new object.
This change makes improvements on both functions.

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Nov 24, 2022
Merged
blathers-crl bot pushed a commit that referenced this issue Nov 24, 2022
@maryliag
util: improve allocation of combine functions
b786cc0 
Part Of #89918

Previously, the functions `CombineUniqueString` and
`CombineUniqueInt64` were always allocating a new object.
This change makes improvements on both functions.

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Nov 24, 2022
Merged
@yuzefovich
Copy link
Member

A user in the cockroachdb slack channel shared a heap profile from 22.2.0 which is very interesting if we look at alloc objects or alloc space samples. ssmemstorage shows up noticeably there.
profile (1).pb.gz

@matthewtodd matthewtodd mentioned this issue Jan 4, 2023
Closed
@matthewtodd
Copy link
Contributor

@maryliag let me hand this back over to you for re-triage. There's some clear low-hanging fruit, just needs a little attention.

@matthewtodd matthewtodd assigned maryliag and unassigned matthewtodd May 16, 2023
@maryliag maryliag removed their assignment May 16, 2023
@dhartunian dhartunian mentioned this issue Aug 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@matthewtodd @maryliag @ajwerner @yuzefovich @kevin-v-ngo