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

Replace fmt.Sprintf's with strconv.Format's #1786

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

austinvazquez
Copy link
Contributor

@austinvazquez austinvazquez commented Jun 26, 2023

What type of PR is this?

  • Non-functional (performance)

What this PR does / why we need it:

This change swaps the ToString implementation under the hood of flag_[bool|float|int|uint].go to use strconv package instead of fmt package.

This is a non-functional change to optimize the ToString operation for [bool|float|int|uint] flags.

Which issue(s) this PR fixes:

None

Testing

var numToConvert uint64 = 100000000

func BenchmarkFmt(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_ = fmt.Sprintf("%d", numToConvert)
	}
}

func BenchmarkStrconv(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_ = strconv.FormatUint(numToConvert, 10)
	}
}
BenchmarkFmt-8           7687989               154.1 ns/op
BenchmarkStrconv-8      21892513                52.77 ns/op

Release Notes

No user facing changes.

@austinvazquez austinvazquez temporarily deployed to frogbot June 26, 2023 15:34 — with GitHub Actions Inactive
@austinvazquez austinvazquez marked this pull request as ready for review June 26, 2023 15:39
@austinvazquez austinvazquez requested a review from a team as a code owner June 26, 2023 15:39
@github-actions
Copy link

What is Frogbot?

@dearchap
Copy link
Contributor

@austinvazquez think you can update flag_bool and flag_float as well to use this ? Thanks

@austinvazquez austinvazquez temporarily deployed to frogbot June 26, 2023 19:53 — with GitHub Actions Inactive
@austinvazquez austinvazquez changed the title Replace fmt.Sprintf's with strconv.Format's for itoa Replace fmt.Sprintf's with strconv.Format's Jun 26, 2023
@austinvazquez
Copy link
Contributor Author

@austinvazquez think you can update flag_bool and flag_float as well to use this ? Thanks

Oh great catch, pushed changes + updated messaging.

@github-actions
Copy link

What is Frogbot?

@dearchap
Copy link
Contributor

@austinvazquez Excellent thank you for the quick turnaround

@dearchap dearchap merged commit 4a9488f into urfave:main Jun 26, 2023
@austinvazquez austinvazquez deleted the optimize-itoa branch June 26, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants