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

Investigate using a zstd library consistent with Lotus #850

Closed
masih opened this issue Jan 27, 2025 · 3 comments
Closed

Investigate using a zstd library consistent with Lotus #850

masih opened this issue Jan 27, 2025 · 3 comments
Assignees

Comments

@masih
Copy link
Member

masih commented Jan 27, 2025

Lotus uses Datadog/zstd (which internally wraps C libraries) but go-f3 uses a pure-go implementation of zstd.

The C version is faster, specifically for non-streaming version. Ideally, we want to use a consistent dependency across both Lotus and go-f3 anyway.

  • Add benchmarks to measure the performace between the two.
  • Switch go-f3 to DataDog dependency.
  • While at it, upgrade Lotus dependency to the latest version too.

Thanks to @ribasushi for pointing this out 🙏

@github-project-automation github-project-automation bot moved this to Todo in F3 Jan 27, 2025
@masih masih self-assigned this Jan 27, 2025
@masih masih added this to the M2: Mainnet Passive Testing milestone Jan 27, 2025
masih added a commit that referenced this issue Jan 27, 2025
Use `DataDog/zstd` library for:
* a more performant implementation
* consistent dependency with the one used in Lotus

See Benchamark results below.

Before:

```
BenchmarkCborEncoding
BenchmarkCborEncoding-12    	  467352	      2376 ns/op	   14680 B/op	      10 allocs/op
BenchmarkCborDecoding
BenchmarkCborDecoding-12    	  104410	     11347 ns/op	   14944 B/op	      27 allocs/op
BenchmarkZstdEncoding
BenchmarkZstdEncoding-12    	  286735	      3897 ns/op	   46748 B/op	      12 allocs/op
BenchmarkZstdDecoding
BenchmarkZstdDecoding-12    	  110794	     10783 ns/op	   28512 B/op	      28 allocs/op
```

After:

```
BenchmarkCborEncoding
BenchmarkCborEncoding-12    	  439345	      2436 ns/op	   14680 B/op	      10 allocs/op
BenchmarkCborDecoding
BenchmarkCborDecoding-12    	  105511	     11395 ns/op	   14944 B/op	      27 allocs/op
BenchmarkZstdEncoding
BenchmarkZstdEncoding-12    	  247182	      4790 ns/op	   28248 B/op	      11 allocs/op
BenchmarkZstdDecoding
BenchmarkZstdDecoding-12    	  113475	     10694 ns/op	   20617 B/op	      28 allocs/op
```

Fixes: #850 #849
@masih masih moved this from Todo to In review in F3 Jan 27, 2025
masih added a commit that referenced this issue Jan 27, 2025
Use `DataDog/zstd` library for:
* a more performant implementation
* consistent dependency with the one used in Lotus

See Benchamark results below.

Before:

```
BenchmarkCborEncoding
BenchmarkCborEncoding-12    	  467352	      2376 ns/op	   14680 B/op	      10 allocs/op
BenchmarkCborDecoding
BenchmarkCborDecoding-12    	  104410	     11347 ns/op	   14944 B/op	      27 allocs/op
BenchmarkZstdEncoding
BenchmarkZstdEncoding-12    	  286735	      3897 ns/op	   46748 B/op	      12 allocs/op
BenchmarkZstdDecoding
BenchmarkZstdDecoding-12    	  110794	     10783 ns/op	   28512 B/op	      28 allocs/op
```

After:

```
BenchmarkCborEncoding
BenchmarkCborEncoding-12    	  439345	      2436 ns/op	   14680 B/op	      10 allocs/op
BenchmarkCborDecoding
BenchmarkCborDecoding-12    	  105511	     11395 ns/op	   14944 B/op	      27 allocs/op
BenchmarkZstdEncoding
BenchmarkZstdEncoding-12    	  247182	      4790 ns/op	   28248 B/op	      11 allocs/op
BenchmarkZstdDecoding
BenchmarkZstdDecoding-12    	  113475	     10694 ns/op	   20617 B/op	      27 allocs/op
```

Fixes: #850 #849
masih added a commit that referenced this issue Jan 28, 2025
Pool the decode buffers and set strict max cap allocation for zstd
decompressor.

See Benchamark results below.

Before:

```
BenchmarkCborEncoding
BenchmarkCborEncoding-12    	  467352	      2376 ns/op	   14680 B/op	      10 allocs/op
BenchmarkCborDecoding
BenchmarkCborDecoding-12    	  104410	     11347 ns/op	   14944 B/op	      27 allocs/op
BenchmarkZstdEncoding
BenchmarkZstdEncoding-12    	  286735	      3897 ns/op	   46748 B/op	      12 allocs/op
BenchmarkZstdDecoding
BenchmarkZstdDecoding-12    	  110794	     10783 ns/op	   28512 B/op	      28 allocs/op
```

After:

```
BenchmarkCborEncoding
BenchmarkCborEncoding-12    	  436754	      2383 ns/op	   14680 B/op	      10 allocs/op
BenchmarkCborDecoding
BenchmarkCborDecoding-12    	  106809	     11280 ns/op	   14944 B/op	      27 allocs/op
BenchmarkZstdEncoding
BenchmarkZstdEncoding-12    	  294043	      3918 ns/op	   46746 B/op	      12 allocs/op
BenchmarkZstdDecoding
BenchmarkZstdDecoding-12    	  114854	     10747 ns/op	   18314 B/op	      27 allocs/op
```

Fixes: #850 #849
@masih
Copy link
Member Author

masih commented Jan 28, 2025

Cross posting lotus benchmark results for visibility: filecoin-project/lotus#12852 (comment)

@masih
Copy link
Member Author

masih commented Jan 29, 2025

Lotus PR to switch to pure go decoder, same as the one currently used by F3: filecoin-project/lotus#12857

Closing this as there are no more actionables here.

@masih masih closed this as completed Jan 29, 2025
@github-project-automation github-project-automation bot moved this from In review to Done in F3 Jan 29, 2025
@BigLep
Copy link
Member

BigLep commented Jan 29, 2025

Thanks for driving this to close with data @masih. Nice work!

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

No branches or pull requests

2 participants