8000 Improve ToString/ToStringE performance by ganigeorgiev · Pull Request #244 · spf13/cast · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

Improve ToString/ToStringE performance #244

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
Merged
merged 1 commit into from
May 29, 2025
Merged

Improve ToString/ToStringE performance #244

merged 1 commit into from
May 29, 2025

Conversation

ganigeorgiev
Copy link
Contributor

Currently ToString/ToStringE always attempts to perform pointer dereference before the type checks which adds significant overhead when casting non-pointer values (for pointers it is also not very optimal because the reflection of errorType and fmtStringerType would be evaluated on every ToString/ToStringE call).

This PR:

  • moves the pointer check in the default clause and calls ToStringE with its underlying value
  • updates the existing tests with equivalent checks for pointers

Below are some basic benchmark results from my local environment (the best from 3 consequent runs):

  1. Plain value

    func BenchmarkToString(b *testing.B) {
    for range b.N {
        cast.ToString("test")
    }
}
    // Old:
BenchmarkToString-4     19877439          60.69 ns/op

// New:
BenchmarkToString-4     282848110         4.364 ns/op

  2. Pointer

    func BenchmarkToString(b *testing.B) {
    v := "test"
    for range b.N {
        cast.ToString(&v)
    }
}
    // Old:
BenchmarkToString-4      4797831         287.2 ns/op

// New:
BenchmarkToString-4     10021204         216.1 ns/op

@CLAassistant
Copy link
CLAassistant commented May 5, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

sagikazarmark
sagikazarmark approved these changes May 29, 2025
View reviewed changes
Copy link
Collaborator
@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@sagikazarmark sagikazarmark merged commit 9354fd3 into spf13:master May 29, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagikazarmark sagikazarmark sagikazarmark approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ganigeorgiev @CLAassistant @sagikazarmark
0