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

cmd: use errrors.New instead of empty fmt.Errorf #27329

Merged
merged 1 commit into from
May 24, 2023

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented May 23, 2023

Seems errors.New is super fast then the empty fmt.Errorf, here is an benchmark test case from https://blog.imberkay.com/errorsnew-vs-fmterrorf

package main

import (
    "errors"
    "fmt"
    "time"
)

func testErrors() error {
    return errors.New("test errors package")
}

func testFmt() error {
    return fmt.Errorf("test fmt package")
}

func main() {
    repeat := 100000000
    start := time.Now()
    for i := 0; i < repeat; i++ {
        testErrors()
    }
    duration := time.Since(start)
    fmt.Println("errors.New", duration)

    start = time.Now()
    for i := 0; i < repeat; i++ {
        testFmt()
    }
    duration = time.Since(start)
    fmt.Println("fmt.Errorf", duration)
}

In my macbook air m1, the result as below:

$ go version
go version go1.20.3 darwin/arm64

$ go run t.go

errors.New 63.858083ms
fmt.Errorf 5.240530083s

Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa jsvisa requested a review from holiman as a code owner May 23, 2023 16:19
@holiman
Copy link
Contributor

holiman commented May 23, 2023

I played around a bit with your example code.

It's not really that errors.New is awesome, or that fmt.Errorf is slow, the reason is that string formatting takes time (even if there's nothing to format), and also that the errors.New is optimized away.

package main

import (
	"errors"
	"fmt"
	"time"
)

func testErrors(txt string) error {
	return errors.New(txt)
}

func testFmt(txt string) error {
	return fmt.Errorf(txt)
}

func main() {
	repeat := 100000000
	start := time.Now()
	var a, b error
	var c string
	for i := 0; i < repeat; i++ {
		a = testErrors("test")
	}
	duration := time.Since(start)
	fmt.Println("errors.New", duration)

	start = time.Now()
	for i := 0; i < repeat; i++ {
		b = fmt.Errorf("test")
	}
	duration = time.Since(start)
	fmt.Println("fmt.Errorf", duration)
	//
	start = time.Now()
	for i := 0; i < repeat; i++ {
		c = fmt.Sprintf("test")
	}
	duration = time.Since(start)
	fmt.Println("fmt.Sprintf", duration)
	fmt.Printf(c)
	fmt.Printf(a.Error())
	fmt.Printf(b.Error())
}

Output:

errors.New 4.762584711s
fmt.Errorf 11.254008468s
fmt.Sprintf 6.893002654s

So ~5s is added to errors.New if we prevent the compiler from removing the entire call, by catching the returnvalue.
And the string-formatting accounts for ~7s. So the ratio between errors.New and fmt.Errorf, for strings without args, are closer to 5:12 than 0.063:5

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM, it's the right thing to do, even if the article is a bit flawed and the results are is a bit misleading

@jsvisa
Copy link
Contributor Author

jsvisa commented May 24, 2023

So ~5s is added to errors.New if we prevent the compiler from removing the entire call, by catching the returnvalue.

Thanks for this tip. It seems that the return value of errors.New is not being used, so the compiler has optimized this line of code.

Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@s1na s1na merged commit e9c3183 into ethereum:master May 24, 2023
@s1na s1na modified the milestones: 1.12.1, 1.11.7 May 24, 2023
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@jsvisa jsvisa deleted the cmd-errorf branch March 8, 2024 07:28
gzliudan added a commit to XinFinOrg/XDPoSChain that referenced this pull request Jun 14, 2024
WIP: use errrors.New instead of empty fmt.Errorf (ethereum#27329)
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