[marcus-pousette-hp]

Before:

remote bitfield gains bits past _remoteContiguousLength
-> _remoteContiguousLength stays put
-> every remoteContiguousLength read rescans the same run

After:

onbitfield / onrange change the remote bitfield
-> move _remoteContiguousLength to the first missing bit
-> reads start from there

Also updates the remote contiguous length hint for any range from 0 past the current hint, since _remoteContiguousLength can already be ahead of it. broadcastRange now also skips some ranges the remote already has.

Should not merge before #923.

Local bench (median of 5 runs). 10k remoteContiguousLength reads over a 50M-block extent built from range messages ({0,1} then {1,N-1}):

before: ~279 ms
after:  ~2.7 ms

Building the extent costs the same on both.

PR was Co-authored with AI.

[lejeunerenard]

Having a getter with side effects is tenuous as getters imply only reading values.

Can we apply these checks when the remote bitfield is mutated (onbitfield & onrange)? This will also ensure the value is updated when we have more info from the remote. Otherwise accessing _remoteContiguousLength directly will have less information than if calling .remoteContiguousLength. We also could explore replacing _remoteContiguousLength to be .remoteContiguousLength, though maybe should be a separate PR to keep scope simple and incremental.

Regarding that, could you add a test for a scenario where onbitfield updates _remoteContigousLength and later a broadcastRange() is called for the same peer but is skipped via the check here:

hypercore/lib/replicator.js

Line 650 in d53c83b

if (this._remoteContiguousLength >= start + length) return

[lejeunerenard]

Setting the fork isn't necessary as they both start at 0.

[lejeunerenard]

Suggested change

	if (has) this._clearLocalRange(rangeStart, rangeEnd - rangeStart)
	if (has && start <= this._remoteContiguousLength && this._remoteContiguousLength < rangeEnd) {
	this._remoteContiguousLength = rangeEnd
	}

We can slightly optimize here by presetting the _remoteContiguousLength to rangeEnd if it only adds to the range covered by _remoteContiguousLength`. Then the scanning of the bitfield can jump over the range just set above.

[lejeunerenard]

We also might want a test for how the remote contiguous could bridge beyond rangeEnd if the bitfield is contiguous afterwards.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR caches remote contiguous lengths after bitfield and range updates. It also lets later prefix announcements update the stored hint when the peer cache is already ahead.

• Remote bitfield updates keep the verified contiguous length cached.

_{Reviews (3) · Last reviewed commit: "Move remote contiguous length to the end..." · Reviewed by Greptile}

[marcus-pousette-hp]

@greptile can you do a performance review? micro bench but also theorize/estimate how this perf fix will benefit common usage patterns vs main . Answer with a breakdown of cpu and mem usage

[marcus-pousette-hp]

@greptile what do you think about the test coverage of this PR?

[greptile-apps]

The coverage is solid for the main regression: the new tests exercise both bitfield and range announcements, including out-of-order ranges that later bridge into a longer contiguous prefix. They also check the persisted hint behavior and that an already-known range is not retransmitted.

I’d still add negative-path coverage before calling it comprehensive:

• Verify a gap or unset bit prevents the cached contiguous length from advancing past it.
• Verify a bitfield overwrite/clear or truncation can reduce the cached length, rather than leaving it stale in the other direction.
• Exercise the relevant range/drop or fork-mismatch cases so the cache and persisted hint remain consistent when those conditions apply.

The tests call the peer handlers directly, so a protocol-level integration test would add confidence, but it’s not essential if those handlers are the intended unit boundary. I wouldn’t put a timing threshold in the unit suite; performance measurements are better kept in a benchmark. Overall: good targeted regression coverage, with invalidation and negative cases as the main gaps.

[marcus-pousette-hp]

@greptile why did you not respond to #810 (comment) ?

[greptile-apps]

Sorry about that. I did post a performance review in the PR-level summary a few minutes after your comment, including CPU, memory, and benchmark caveats, but I didn’t reply directly to your request. That made it look unanswered. It wasn’t intentional; I should have responded in the thread.

[marcus-pousette-hp]

@greptile please answer then hat the perf result where from your last review

[greptile-apps]

The 51.7 ms before and 2.26 ms after figures came from the benchmark in the PR description. I did not run that benchmark independently. My CPU and memory breakdown in the previous review was a code-based estimate, not measured profiling data; I should have made that distinction clearer.

[marcus-pousette-hp]

@greptile please run it

[marcus-pousette-hp]

Found one more thing while testing this. RemoteBitfieldSegment.findFirst(false, pos) returns pagesLength * 32768 when pos is past the segment's last allocated page, so it can return less than pos. On main that only affects the getter, so at worst we send an extra want. With this PR the refresh writes it into _remoteContiguousLength: once the remote has a bit in a lower page, a range from 0 past it gets cut back to that page boundary, and the blocks after it are never requested from that peer.

#923 fixes findFirst, so this should land after it