-1

When I write benchmark test for my algorithm, I was confused by an problem!

My test code detail was pushed to github and I copy it to here and add some comments.

https://github.com/hidstarshine/Algorithm/blob/master/leet/problem24_test.go

var TDBenchmarkSwapPairs1 *leet.ListNode

// This function may be not good, it should be init()?
func FTDBenchmarkSwapPairs1() {
    TDBenchmarkSwapPairs1 = &leet.ListNode{
        Val:  0,
        Next: nil,
    }
    changeNode := TDBenchmarkSwapPairs1
    for i := 1; i < 100; i++ {
        changeNode.Next = &leet.ListNode{
            Val:  i,
            Next: nil,
        }
        changeNode = changeNode.Next
    }
}

func BenchmarkSwapPairs1(b *testing.B) {
    FTDBenchmarkSwapPairs1() // problem is here
    for i := 0; i < b.N; i++ {
        leet.SwapPairs1(TDBenchmarkSwapPairs1)
    }
}

In the problem line, I call the FTDBenchmarkSwapPairs1(FTD means fill test data) to initialize the data.

Then something amzing happen, the BenchmarkSwapPairs1 seems to run in many goroutine.

So the concurrency bring the data race and due to the SwapPairs1 special logical the debug was in a mess.

SwapPairs1 will change the Next in the ListNode.

Then I want to move the BenchmarkSwapPairs1 to the block of for to solve this.

But the data race seems still unsolve and the benchmark test is no meaning because of the time of initialization.

I judge the argorithm on leetcode and get accepted!

Q: How could I solve this elegantly? Need a good idea!


NEW @Jimb

I add just add some debug info then it panic. I also think it will don't have data race at the begining.

I make the assumption when I saw the panic!

package leet_test

import (
    "fmt"
    "testing"

    "github.com/hidstarshine/Algorithm/leet"
)

var TDBenchmarkSwapPairs1 *leet.ListNode

func FTDBenchmarkSwapPairs1() {
    TDBenchmarkSwapPairs1 = &leet.ListNode{
        Val:  0,
        Next: nil,
    }
    changeNode := TDBenchmarkSwapPairs1
    for i := 1; i < 100; i++ {
        changeNode.Next = &leet.ListNode{
            Val:  i,
            Next: nil,
        }
        changeNode = changeNode.Next
    }
    AnotherChangeNode := TDBenchmarkSwapPairs1
    for AnotherChangeNode != nil {
        fmt.Println(AnotherChangeNode)
        AnotherChangeNode = AnotherChangeNode.Next
    }
}

func BenchmarkSwapPairs1(b *testing.B) {
    FTDBenchmarkSwapPairs1()
    for i := 0; i < b.N; i++ {
        fmt.Println(TDBenchmarkSwapPairs1.Next)
        fmt.Println(TDBenchmarkSwapPairs1.Next.Next)
        fmt.Println(TDBenchmarkSwapPairs1.Next.Next.Next)
        fmt.Println(TDBenchmarkSwapPairs1.Next.Next.Next.Next)
        leet.SwapPairs1(TDBenchmarkSwapPairs1)
    }
}


Panic Info( imprtant )

more...

&{98 0xc000044ac0}
&{99 <nil>}
&{1 0xc000044270}
&{2 0xc0000444a0}
&{3 0xc0000444c0}
&{4 0xc0000444d0}

Some system info

&{15 0xc000044ae0}
&{2 0xc000044bd0}
&{17 0xc000044b00}
&{4 0xc000044bf0}
&{17 0xc000044ae0}

Unorderd message

<nil>
&{4 0xc000044ae0}
&{2 <nil>}
<nil>
panic: runtime error: invalid memory address or nil pointer dereference // why
[signal 0xc0000005 code=0x0 addr=0x8 pc=0xbefb20]
Crowman
  • 25,242
  • 5
  • 48
  • 56
axlis
  • 395
  • 1
  • 3
  • 13
  • A benchmark does not run in multiple goroutines, unless you start multiple goroutines yourself. Why do you think you have a race condition? Please create a [mre] showing what problem you are trying to solve. – JimB Jul 15 '21 at 13:58
  • @JimB I add some debug info show the strange thing? I can't find any explanation for this. – axlis Jul 15 '21 at 14:33
  • You are showing a nil pointer dereference, and the stack trace you didn't include will show where exactly in the code that happens. There is no race here. – JimB Jul 15 '21 at 14:38
  • @JimB thank for your advice, the problem was found! – axlis Jul 15 '21 at 15:07

1 Answers1

5

If you have multiple benchmark functions you probably don't want them to interfere with each other's data, so use a local variable instead of a (shared) package-level variable.

You can use *B.ResetTimer to remove the setup time from the overall benchmark running time.

func BenchmarkSwapPairs1(b *testing.B) {
    root := &leet.ListNode{
        Val:  0,
        Next: nil,
    }
    changeNode := root
    for i := 1; i < 10000; i++ {
        changeNode.Next = &leet.ListNode{
            Val:  i,
            Next: nil,
        }
        changeNode = changeNode.Next
    }
    b.ResetTimer()

    for i := 0; i < b.N; i++ {
        root = leet.SwapPairs1(root)
    }
}
bcmills
  • 4,391
  • 24
  • 34