urfave / cli

A simple, fast, and fun package for building command line apps in Go
https://cli.urfave.org
MIT License
22.22k stars 1.7k forks source link

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

Closed austinvazquez closed 1 year ago

austinvazquez commented 1 year ago

What type of PR is this?

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.

github-actions[bot] commented 1 year ago

What is Frogbot?

dearchap commented 1 year ago

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

austinvazquez commented 1 year ago

@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[bot] commented 1 year ago

What is Frogbot?

dearchap commented 1 year ago

@austinvazquez Excellent thank you for the quick turnaround