Skip to content

Conversation

@MichaelMure
Copy link
Contributor

When doing a Put, the containing directory is always created. If it already exist,
no error bubbles up from makeDirNoSync and later this directly is always synced on disk
in makeDir.

This generate FS operations which are almost always unnecessary as the supporting folders
are quickly all created.

When doing a Put, the containing directory is always created. If it already exist,
no error bubbles up from makeDirNoSync and later this directy is always synched on disk
in makeDir.

This generate FS operations which are almost always unnecessary as the supporting folders
are quickly all created.
@MichaelMure
Copy link
Contributor Author

MichaelMure commented May 9, 2022

I tried to benchmark both version but the results vary widely from one run to another, even when using flags like -benchtime 100x.

My benchmark code was:

func BenchmarkPut(b *testing.B) {
	temp, cleanup := tempdir(b)
	defer cleanup()
	// defer checkTemp(b, temp)

	fs, err := flatfs.CreateOrOpen(temp, flatfs.NextToLast(2), false)
	if err != nil {
		b.Fatalf("New fail: %v\n", err)
	}
	defer fs.Close()

	b.ResetTimer()
	b.ReportAllocs()

	for i := 0; i < b.N; i++ {
		err = fs.Put(bg, datastore.NewKey("QUUX"), []byte("foobar"))
		if err != nil {
			b.Fatalf("Put fail: %v\n", err)
		}
	}
}

@guseggert
Copy link
Contributor

Are you saying that there's no measurable difference in performance? If that's the case, I'd prefer to keep things consistent / make less assumptions, makes the code easier to maintain.

@MichaelMure
Copy link
Contributor Author

MichaelMure commented May 13, 2022

I did another benchmarking on NFS:

Before:

BenchmarkPut-12    	      19	  90327492 ns/op	    2442 B/op	      39 allocs/op
BenchmarkPut-12    	      20	  60113199 ns/op	    2934 B/op	      39 allocs/op
BenchmarkPut-12    	      19	 144812107 ns/op	    3016 B/op	      40 allocs/op
BenchmarkPut-12    	      20	  60713097 ns/op	    2507 B/op	      39 allocs/op
BenchmarkPut-12    	      18	  57389155 ns/op	    3008 B/op	      39 allocs/op
BenchmarkPut-12    	      10	 135447145 ns/op	    3417 B/op	      42 allocs/op
BenchmarkPut-12    	      19	  72482237 ns/op	    2962 B/op	      39 allocs/op
BenchmarkPut-12    	      20	  86261195 ns/op	    2867 B/op	      39 allocs/op
AVG                                                    88443203.375		2894.125		39.5

After:

BenchmarkPut-12    	      22	  61761422 ns/op	    2660 B/op	      36 allocs/op
BenchmarkPut-12    	      20	  81016432 ns/op	    2343 B/op	      36 allocs/op
BenchmarkPut-12    	      21	  79589929 ns/op	    2761 B/op	      36 allocs/op
BenchmarkPut-12    	      10	 121222088 ns/op	    2496 B/op	      39 allocs/op
BenchmarkPut-12    	      24	  57701723 ns/op	    2670 B/op	      35 allocs/op
BenchmarkPut-12    	      21	  47750888 ns/op	    2346 B/op	      36 allocs/op
BenchmarkPut-12    	      25	  82605228 ns/op	    2326 B/op	      35 allocs/op
BenchmarkPut-12    	      19	  84322739 ns/op	    2815 B/op	      36 allocs/op
AVG                                                    76996306.125		2552.125		36.125

13% faster, 12% less allocation (bytes), 9% less allocation (count) ... but as you can see, the benchmark varies.

@guseggert guseggert merged commit cebe9d0 into ipfs:master May 24, 2022
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.

2 participants