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

Inefficient use of sliceOfValues in profiles data #11281

Closed
bogdandrutu opened this issue Sep 26, 2024 · 5 comments · Fixed by #11339
Closed

Inefficient use of sliceOfValues in profiles data #11281

bogdandrutu opened this issue Sep 26, 2024 · 5 comments · Fixed by #11339
Milestone
Profiling support

Comments

@bogdandrutu
Copy link
Member

bogdandrutu commented Sep 26, 2024 •
edited
Loading

We should never use sliceOfValues for large objects (more than ~32B). See #10195 where everything is marked as "nullable" which will make re-allocation which happens quite often during unmarshaling very expensive because copies lots of data.

One concrete example MappingSlice or LocationSlice should not be a value slice. But someone needs to re-evaluate all of them.
@bogdandrutu
Copy link
Member Author

Assigning to @mx-psi based on the maintainer who merged the PR.

@mx-psi mx-psi removed their assignment Oct 1, 2024
@mx-psi mx-psi added this to the Profiling support milestone Oct 1, 2024
@mx-psi
Copy link
Member

mx-psi commented Oct 1, 2024

I am not going to work on this any time soon since I am focusing on Collector 1.0, so I am un-assigning myself to make that clear

@bogdandrutu
Copy link
Member Author

@mx-psi I understand that, but we need to make it clear that approvers/maintainers that are reviewing PRs become the owners of the code, so hence you need to find someone to help you with this.

@narcis96
Copy link
Contributor

narcis96 commented Oct 3, 2024 •
edited
Loading

Hi @bogdandrutu and @mx-psi ! I'm interested in working on this! Would you be able to guide me on what needs to be done?

@dmathieu
Copy link
Member

dmathieu commented Oct 3, 2024

As the person who's been working on profiles so far, I'm looking into this.

@dmathieu dmathieu mentioned this issue Oct 3, 2024
Merged
jackgopack4 pushed a commit to jackgopack4/opentelemetry-collector that referenced this issue Oct 8, 2024
@dmathieu @jackgopack4
Use slices of pointers in pprofile (open-telemetry#11339)
8580444 
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

All these slices of values have a potential to store large amounts of
data, and should therefore be slices of pointers.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes open-telemetry#11281

cc @mx-psi @bogdandrutu
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this issue Dec 19, 2024
@dmathieu @HongChenTW
Use slices of pointers in pprofile (open-telemetry#11339)
189ead7 
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

All these slices of values have a potential to store large amounts of
data, and should therefore be slices of pointers.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes open-telemetry#11281

cc @mx-psi @bogdandrutu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Profiling support
Development

Successfully merging a pull request may close this issue.

Use slices of pointers in pprofile dmathieu/opentelemetry-collector
4 participants
@dmathieu @bogdandrutu @mx-psi @narcis96