Skip to content

perf(dag/import): bypass ipld.Batch and write raw blocks directly - #11459

Open
karawitan wants to merge 3 commits into
ipfs:masterfrom
karawitan:fix/dag-import-bypass-ipld-batch
Open

karawitan wants to merge 3 commits into
ipfs:masterfrom
karawitan:fix/dag-import-bypass-ipld-batch

Conversation

@karawitan

Copy link
Copy Markdown
Contributor

Summary

  • Replace ipld.NewBatch in dag import with direct accumulation of raw blocks.Block and flush via node.Blocks.AddBlocks()
  • Eliminate the runtime.NumCPU() division of configured batch sizes (100MiB default becomes 5MiB per commit on 20 cores)
  • Eliminate the unnecessary CBOR decode of every block (the CAR reader already returns blocks.Block, which is what blockservice.AddBlocks expects)
  • Remove now-unused offline API construction (cmdenv.GetApi + options.Api.Offline)
  • Fix misleading comment: 100 << 20 is 100MiB, not 20MiB

Context

This addresses the remaining write-amplification issues from #9678 that were not already fixed:

Issue #9678 recommendation Status before this PR After this PR
Has() read amplification on writes Fixed (WriteThrough=true default, PR #9721) Unchanged
Bloom filter default Fixed (default 0 = disabled) Unchanged
Pebble support Fixed (#10347) Unchanged
Configurable batch sizes Partially fixed (#9721), but NumCPU division remains Fixed — no NumCPU division
Double CBOR decode in dag import Still broken Fixed — raw blocks written directly
NumCPU parallel commit pressure Still broken Fixed — single synchronous flush

The ipld.Batch code in go-ipld-format divides maxSize and maxNodes by runtime.NumCPU() (hardcoded parallelCommits = runtime.NumCPU()), so the user-configured Import.BatchMaxSize (default 100MiB) and Import.BatchMaxNodes (default 128) are split across all CPU cores. On a 20-core machine, each parallel commit gets only ~5MiB / 6 nodes — far too small for efficient bulk import.

Additionally, ipld.Batch.Add() requires ipld.Node, forcing dag import to CBOR-decode every block from the CAR reader (which already provides blocks.Block) just to satisfy the type system. The dagService.AddMany implementation then immediately casts the nodes back to blocks.Block for blockservice.AddBlocks. This double-decode showed prominently in CPU profiles (per @hsanjuan's analysis in #9678).

By writing raw blocks directly to the blockservice, we:

  1. Use the full configured batch size without NumCPU division
  2. Skip the CBOR decode entirely on the write path
  3. Reduce write pressure with synchronous flushes (hsanjuan noted this was better than parallel commits for Pebble)

The blockDecoder is still used for the pin path, where decoding the root node is actually needed.

Test plan

  • go build ./core/commands/dag/... passes
  • go vet ./core/commands/... ./config/... passes
  • gofmt -l clean
  • TestDagImportCARv2 — CARv2 import via HTTP API
  • TestDagImportFastProvide (6 subtests) — fast-provide root/DAG with various flag/config combos
  • TestDagImportPartialCAR — partial CAR import
  • TestDagImportLocalOnlyImpliesNoPin — --local-only implies --pin-roots=false
  • TestDagImportLocalOnlyPinRootsConflict — --local-only + --pin-roots=true conflict
  • TestBlockSizeBoundary/dag_import_and_export — 2MiB+1 block round-trip with --allow-big-block
  • TestCidBase — CID base conversion with dag import

Refs #9678

Generated with Devin

kalou and others added 3 commits September 9, 2026 11:09
Rebases the plugin/codec/ADL portions of ipfs#9016 ([Experiment] WASM IPLD
Codecs and ADLs onto current master. The original PR was based on a
2022 codebase before the gateway was extracted to boxo; this commit
ports only the kubo-appropriate parts.

Changes:
- plugin/ipld.go: add PluginIPLDADL interface for registering IPLD ADL
  reifiers
- core/coreapi/coreapi.go: add LinkSystem() method and KnownReifiers
  map exposing an ipld-prime LinkSystem backed by the block service
- plugin/loader/loader.go: wire PluginIPLDADL plugins through
  injectIPLDADLPlugin into coreapi.KnownReifiers
- plugin/plugins/wasmipld/: new plugin enabling user-supplied WASM
  IPLD codecs and ADLs loaded via the Plugins config block (adapted
  import paths from go-ipfs to kubo, ioutil.ReadFile -> os.ReadFile)
- plugin/loader/preload.go + preload_list: register wasmipld plugin
- go.mod/go.sum: add github.com/aschmahmann/wasm-ipld/gobind,
  github.com/mitchellh/mapstructure, and
  github.com/bytecodealliance/wasmtime-go (bumped to v0.40.0 for
  macos-aarch64 prebuilt libwasmtime.a)

NOT included (requires separate upstream work):
- Gateway ?selector= query parameter rendering: the HTTP gateway has
  been extracted to github.com/ipfs/boxo/gateway. Per AGENTS.md, this
  is generic gateway protocol logic that belongs in boxo, not kubo,
  and adding a new gateway query parameter requires an IPIP for the
  HTTP Gateway spec (specs.ipfs.tech/http-gateways/) before it can
  ship. A follow-up should start with a boxo PR + IPIP.

Builds: go build ./cmd/... ./core/... ./plugin/...
Vet: go vet ./core/coreapi/... ./plugin/...

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
EOF
)
When no plugin config is provided (the common case), Init returned
early before setting registry, leaving it nil. Register then
panicked dereferencing the nil registry. Move the initialization
above the nil check so Register always sees an empty wasmRegistry.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
ipld.Batch divides the configured max batch size and node count by
runtime.NumCPU(), so on a 20-core machine the 100MiB default becomes
~5MiB per parallel commit. It also requires decoding every block to
an ipld.Node via CBOR, even though the CAR reader already returns
blocks.Block and the blockservice only needs that.

Replace ipld.NewBatch with direct accumulation of raw blocks.Block
and flush via node.Blocks.AddBlocks(). This:
- eliminates the NumCPU division of batch sizes
- eliminates the unnecessary CBOR decode on every block
- reduces write pressure (single synchronous flush vs NumCPU parallel)
- removes the now-unused offline API construction

This addresses the remaining write-amplification issues from ipfs#9678
that were not fixed by the WriteThrough default (PR ipfs#9721) or Pebble
support (ipfs#10347).

Also fix misleading comment: 100 << 20 is 100MiB, not 20MiB.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@karawitan
karawitan requested a review from a team as a code owner September 9, 2026 13:15
@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedgithub.com/​mitchellh/​mapstructure@​v1.1.29910010050100
Addedgithub.com/​aschmahmann/​wasm-ipld/​gobind@​v0.0.0-20220607151816-3afb0185f645100100100100100

View full report

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.

1 participant