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

ReadWrite: faster Has() by using the in-memory index instead of reading on disk #393

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

MichaelMure
Copy link
Contributor

Before:
BenchmarkHas-12 205046 6404 ns/op 744 B/op 16 allocs/op

After
BenchmarkHas-12 2354449 481.3 ns/op 160 B/op 3 allocs/op

func BenchmarkHas(b *testing.B) {
	ctx := context.TODO()

	path := filepath.Join(b.TempDir(), "bench-large-v2.car")
	generateRandomCarV2File(b, path, 200<<20) // 10 MiB
	defer os.Remove(path)

	subject, err := blockstore.OpenReadWrite(path, nil)

	c, err := subject.AllKeysChan(ctx)
	require.NoError(b, err)

	var allCids []cid.Cid

	for c2 := range c {
		allCids = append(allCids, c2)
	}

	b.ReportAllocs()
	b.ResetTimer()

	var idx int
	for i := 0; i < b.N; i++ {
		_, _ = subject.Has(ctx, allCids[idx])
		// require.NoError(b, err)
		// require.True(b, has)
		idx = (idx + 1) % len(allCids)
	}
}

@MichaelMure
Copy link
Contributor Author

I keep falling into the same trap. go.mod files act as boundaries for tests, so if your run tests from the root of that package, the whole /v2 is not tested ...

@MichaelMure
Copy link
Contributor Author

MichaelMure commented Mar 15, 2023

Before

BenchmarkHas-8 190216 6368 ns/op 744 B/op 16 allocs/op

After

BenchmarkHas-8 1419169 845.6 ns/op 320 B/op 6 allocs/op

7.5x faster, 2.32x less allocation. That's still 6 allocations due to decoding multihashes and friends, there might be a better way.

…ng on disk

Before:
// BenchmarkHas-8   	  190216	      6368 ns/op	     744 B/op	      16 allocs/op

After
//  BenchmarkHas-8   	 1419169	       845.6 ns/op	     320 B/op	       6 allocs/op

```
func BenchmarkHas(b *testing.B) {
	ctx := context.TODO()

	path := filepath.Join(b.TempDir(), "bench-large-v2.car")
	generateRandomCarV2File(b, path, 200<<20) // 10 MiB
	defer os.Remove(path)

	subject, err := blockstore.OpenReadWrite(path, nil)

	c, err := subject.AllKeysChan(ctx)
	require.NoError(b, err)

	var allCids []cid.Cid

	for c2 := range c {
		allCids = append(allCids, c2)
	}

	b.ReportAllocs()
	b.ResetTimer()

	var idx int
	for i := 0; i < b.N; i++ {
		_, _ = subject.Has(ctx, allCids[idx])
		// require.NoError(b, err)
		// require.True(b, has)
		idx = (idx + 1) % len(allCids)
	}
}
```
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎉 Done
Development

Successfully merging this pull request may close these issues.

2 participants