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

strings: (*Builder).Grow should take benefit from sizeclasses #64833

Closed
mateusz834 opened this issue Dec 21, 2023 · 1 comment
Closed

strings: (*Builder).Grow should take benefit from sizeclasses #64833

mateusz834 opened this issue Dec 21, 2023 · 1 comment
Assignees
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@mateusz834
Copy link
Member

The Grow method on strings.Builder is documented as:

Grow grows b's capacity, if necessary, to guarantee space for another n bytes. After Grow(n), at least n bytes can be written to b without another allocation. If n is negative, Grow panics.

Currently it grows as such:

// grow copies the buffer to a new, larger buffer so that there are at least n
// bytes of capacity beyond len(b.buf).
func (b *Builder) grow(n int) {
	buf := bytealg.MakeNoZero(2*cap(b.buf) + n)[:len(b.buf)]
	copy(buf, b.buf)
	b.buf = buf
}

I think that it is backwards compatible to start using sizeclasses in the strings.Builder, and not waste memory while growing.

@mateusz834 mateusz834 added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 21, 2023
@mateusz834 mateusz834 added this to the Backlog milestone Dec 21, 2023
@mateusz834 mateusz834 self-assigned this Dec 21, 2023
@mateusz834 mateusz834 changed the title strings: strings.Builder.Grow should take benefit from sizeclasses strings: strings.Builder Grow should take benefit from sizeclasses Dec 21, 2023
@mateusz834 mateusz834 changed the title strings: strings.Builder Grow should take benefit from sizeclasses strings: (*Builder).Grow should take benefit from sizeclasses Dec 21, 2023
@gopherbot
Copy link

Change https://go.dev/cl/552135 mentions this issue: strings: make use of sizeclasses in (*Builder).Grow

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 2, 2024
@bcmills bcmills modified the milestones: Backlog, Go1.23 Jan 2, 2024
@bcmills bcmills added FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants