boxo

boxo copied to clipboard

Variadicts in go are just syntactic sugar around passing a slice, that means all go memory reachability rules apply, this force the compiler to heap allocate the variadic slice for virtual call, because the implementation is allowed to leak the slice (and go's interprocedural optimisations do not cover virtuals).

Passing a block without variadic will pass the itab either on the stack or decomposed through registers. Skipping having to allocate a slice.

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.32%. Comparing base (19a402b) to head (730b9bd).

Files with missing lines	Patch %	Lines
blockservice/blockservice.go	50.00%	1 Missing and 1 partial :warning:

@@            Coverage Diff             @@
##             main     #242      +/-   ##
==========================================
- Coverage   60.36%   60.32%   -0.04%     
==========================================
  Files         243      243              
  Lines       30953    30968      +15     
==========================================
- Hits        18684    18682       -2     
- Misses      10612    10626      +14     
- Partials     1657     1660       +3     

Files with missing lines	Coverage Δ
bitswap/bitswap.go	69.23% <100.00%> (+1.78%)	:arrow_up:
bitswap/client/client.go	90.22% <100.00%> (+2.10%)	:arrow_up:
bitswap/server/internal/decision/engine.go	91.44% <ø> (ø)
bitswap/server/server.go	64.42% <100.00%> (+0.50%)	:arrow_up:
exchange/offline/offline.go	88.88% <100.00%> (+1.01%)	:arrow_up:
blockservice/blockservice.go	78.00% <50.00%> (-0.23%)	:arrow_down:

... and 9 files with indirect coverage changes

@Jorropo : I don't see an issue linked to this PR. In the absence of this, can you share more on what's motivating this PR?

@BigLep the PR and Commit bodies:

Variadicts in go are just syntactic sugar around passing a slice, that means all go memory reachability rules apply, this force the compiler to heap allocate the variadic slice for virtual call, because the implementation is allowed to leak the slice (and go's interprocedural optimisations do not cover virtuals). Passing a block without variadic will pass the itab either on the stack or decomposed through registers. Skipping having to allocate a slice.

I can add the explanation about keeping it in-line with our other exchange APIs.

---

This seems like a good usecase to try the idea @guseggert is cooking (some automated code migrator), that would add the NotifyNewBlock method (which redirects to NotifyNewBlocks) automatically if you run it.

2023-04-06 conversation: will defer for now. Want to have clarity on how we handle breaking changes in Boxo.

This is ready to merge. Let's decide if we want this or not.

CI fails when calling s.exchange.NotifyNewBlock(ctx, o) in blockservice.go here, but succeeds when calling s.exchange.NotifyNewBlocks(ctx, o). Ther should be no difference.