Skip to content

fix: replace unsafe currentSubtree.Load().Length() with channel-synce…#641

Open
sugh01 wants to merge 1 commit intobsv-blockchain:mainfrom
sugh01:fix-subtree-sync
Open

fix: replace unsafe currentSubtree.Load().Length() with channel-synce…#641
sugh01 wants to merge 1 commit intobsv-blockchain:mainfrom
sugh01:fix-subtree-sync

Conversation

@sugh01
Copy link
Copy Markdown
Collaborator

@sugh01 sugh01 commented Mar 30, 2026

…d GetCurrentLength() in subtree processor tests

@sugh01
fix: replace unsafe currentSubtree.Load().Length() with channel-synce…
9642d20 
…d GetCurrentLength() in subtree processor tests
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026 •
edited
Loading

🤖 Claude Code Review

Status: Complete

Summary: This PR improves test reliability by replacing unsafe direct atomic loads with the channel-synchronized GetCurrentLength() method. The changes eliminate race conditions in concurrent tests.

Findings: No issues found

What Changed:

  • Replaced stp.currentSubtree.Load().Length() with stp.GetCurrentLength() in test assertions
  • Replaced busy-wait loops (for stp.txCount.Load() < n) with require.Eventually() with proper timeouts
  • Added clear comments explaining the synchronization guarantees
  • Improved test readability and robustness

Why This Matters:
The GetCurrentLength() method synchronizes through the processor's channel-based select loop (SubtreeProcessor.go:740-742), ensuring memory visibility of shared state (chainedSubtrees, currentTxMap). Direct atomic loads bypassed this synchronization, leading to potential flaky test failures.

Technical Quality:
✅ Correct use of channel synchronization for happens-before ordering
✅ Proper timeouts with require.Eventually() (5 seconds)
✅ Clear explanatory comments on memory model guarantees
✅ Follows testify conventions from coding standards
✅ No performance impact (tests only)

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 30, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-641 (294f81d)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 151
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.393µ 1.389µ ~ 0.700
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.58n 61.63n ~ 1.000
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 61.44n 61.55n ~ 0.200
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 61.65n 61.60n ~ 0.500
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 29.43n 29.99n ~ 0.200
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 51.20n 51.60n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 106.9n 106.3n ~ 0.800
MiningCandidate_Stringify_Short-4 257.6n 257.3n ~ 0.400
MiningCandidate_Stringify_Long-4 1.847µ 1.844µ ~ 1.000
MiningSolution_Stringify-4 930.1n 926.2n ~ 0.400
BlockInfo_MarshalJSON-4 1.757µ 1.752µ ~ 0.400
NewFromBytes-4 131.1n 163.0n ~ 0.500
Mine_EasyDifficulty-4 57.87µ 57.87µ ~ 1.000
Mine_WithAddress-4 4.658µ 4.629µ ~ 0.400
BlockAssembler_AddTx-4 0.02787n 0.03139n ~ 0.400
AddNode-4 11.45 10.97 ~ 0.200
AddNodeWithMap-4 11.06 10.96 ~ 1.000
DirectSubtreeAdd/4_per_subtree-4 60.98n 61.17n ~ 0.700
DirectSubtreeAdd/64_per_subtree-4 29.12n 29.02n ~ 0.700
DirectSubtreeAdd/256_per_subtree-4 27.94n 27.92n ~ 0.500
DirectSubtreeAdd/1024_per_subtree-4 26.57n 26.63n ~ 0.700
DirectSubtreeAdd/2048_per_subtree-4 26.21n 26.18n ~ 1.000
SubtreeProcessorAdd/4_per_subtree-4 300.6n 304.1n ~ 0.100
SubtreeProcessorAdd/64_per_subtree-4 300.3n 298.8n ~ 0.400
SubtreeProcessorAdd/256_per_subtree-4 301.3n 300.5n ~ 0.400
SubtreeProcessorAdd/1024_per_subtree-4 296.7n 298.7n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 296.2n 298.8n ~ 0.400
SubtreeProcessorRotate/4_per_subtree-4 302.1n 301.3n ~ 0.700
SubtreeProcessorRotate/64_per_subtree-4 302.4n 299.2n ~ 0.200
SubtreeProcessorRotate/256_per_subtree-4 300.1n 298.3n ~ 1.000
SubtreeProcessorRotate/1024_per_subtree-4 298.3n 298.2n ~ 1.000
SubtreeNodeAddOnly/4_per_subtree-4 66.04n 66.08n ~ 1.000
SubtreeNodeAddOnly/64_per_subtree-4 40.73n 41.95n ~ 0.100
SubtreeNodeAddOnly/256_per_subtree-4 39.73n 39.57n ~ 0.700
SubtreeNodeAddOnly/1024_per_subtree-4 38.16n 38.54n ~ 0.400
SubtreeCreationOnly/4_per_subtree-4 143.6n 144.1n ~ 1.000
SubtreeCreationOnly/64_per_subtree-4 578.4n 571.3n ~ 1.000
SubtreeCreationOnly/256_per_subtree-4 1.982µ 2.012µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 7.161µ 7.032µ ~ 0.700
SubtreeCreationOnly/2048_per_subtree-4 13.25µ 12.66µ ~ 0.100
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 302.4n 300.6n ~ 0.400
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 300.1n 299.6n ~ 1.000
ParallelGetAndSetIfNotExists/1k_nodes-4 905.1µ 908.4µ ~ 0.700
ParallelGetAndSetIfNotExists/10k_nodes-4 1.879m 1.931m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 8.009m 8.260m ~ 0.100
ParallelGetAndSetIfNotExists/100k_nodes-4 15.54m 16.03m ~ 0.100
SequentialGetAndSetIfNotExists/1k_nodes-4 716.2µ 736.5µ ~ 0.100
SequentialGetAndSetIfNotExists/10k_nodes-4 2.987m 3.126m ~ 0.100
SequentialGetAndSetIfNotExists/50k_nodes-4 11.32m 11.74m ~ 0.100
SequentialGetAndSetIfNotExists/100k_nodes-4 21.92m 22.54m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 918.2µ 933.9µ ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 4.937m 5.065m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 18.91m 19.37m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 783.3µ 774.3µ ~ 0.700
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 6.671m 6.611m ~ 0.400
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 42.27m 42.47m ~ 1.000
DiskTxMap_SetIfNotExists-4 4.180µ 4.310µ ~ 0.100
DiskTxMap_SetIfNotExists_Parallel-4 3.865µ 4.007µ ~ 0.100
DiskTxMap_ExistenceOnly-4 442.7n 392.1n ~ 0.200
Queue-4 198.6n 201.6n ~ 0.400
AtomicPointer-4 3.246n 3.670n ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 857.6µ 914.4µ ~ 0.700
ReorgOptimizations/DedupFilterPipeline/New/10K-4 815.8µ 905.5µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 121.8µ 118.4µ ~ 0.100
ReorgOptimizations/AllMarkFalse/New/10K-4 64.99µ 64.27µ ~ 0.200
ReorgOptimizations/HashSlicePool/Old/10K-4 73.16µ 61.39µ ~ 0.200
ReorgOptimizations/HashSlicePool/New/10K-4 11.08µ 10.97µ ~ 0.100
ReorgOptimizations/NodeFlags/Old/10K-4 5.167µ 5.443µ ~ 0.100
ReorgOptimizations/NodeFlags/New/10K-4 1.800µ 1.933µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 11.75m 12.87m ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/100K-4 12.22m 12.35m ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.200m 1.212m ~ 1.000
ReorgOptimizations/AllMarkFalse/New/100K-4 709.9µ 709.6µ ~ 0.700
ReorgOptimizations/HashSlicePool/Old/100K-4 641.1µ 591.2µ ~ 0.200
ReorgOptimizations/HashSlicePool/New/100K-4 210.2µ 207.4µ ~ 0.700
ReorgOptimizations/NodeFlags/Old/100K-4 55.80µ 57.09µ ~ 0.700
ReorgOptimizations/NodeFlags/New/100K-4 20.09µ 20.27µ ~ 0.700
TxMapSetIfNotExists-4 47.04n 46.76n ~ 1.000
TxMapSetIfNotExistsDuplicate-4 38.77n 38.88n ~ 0.400
ChannelSendReceive-4 633.8n 605.1n ~ 0.100
CalcBlockWork-4 493.6n 499.5n ~ 0.100
CalculateWork-4 681.4n 679.7n ~ 1.000
BuildBlockLocatorString_Helpers/Size_10-4 1.371µ 1.540µ ~ 0.200
BuildBlockLocatorString_Helpers/Size_100-4 13.26µ 13.25µ ~ 1.000
BuildBlockLocatorString_Helpers/Size_1000-4 134.4µ 129.5µ ~ 0.100
CatchupWithHeaderCache-4 104.2m 104.2m ~ 0.700
_BufferPoolAllocation/16KB-4 3.273µ 3.239µ ~ 1.000
_BufferPoolAllocation/32KB-4 8.175µ 7.603µ ~ 0.400
_BufferPoolAllocation/64KB-4 16.20µ 16.01µ ~ 0.400
_BufferPoolAllocation/128KB-4 31.60µ 30.75µ ~ 0.200
_BufferPoolAllocation/512KB-4 107.4µ 107.3µ ~ 1.000
_BufferPoolConcurrent/32KB-4 17.63µ 19.29µ ~ 0.100
_BufferPoolConcurrent/64KB-4 28.57µ 30.12µ ~ 0.100
_BufferPoolConcurrent/512KB-4 142.1µ 148.5µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/16KB-4 625.8µ 630.9µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/32KB-4 633.7µ 636.4µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/64KB-4 627.9µ 630.3µ ~ 0.700
_SubtreeDeserializationWithBufferSizes/128KB-4 630.9µ 630.9µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/512KB-4 648.3µ 655.4µ ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/16KB-4 35.23m 35.28m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/32KB-4 35.04m 34.88m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/64KB-4 34.76m 35.32m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/128KB-4 35.10m 35.12m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/512KB-4 34.98m 34.93m ~ 1.000
_PooledVsNonPooled/Pooled-4 733.4n 737.5n ~ 0.700
_PooledVsNonPooled/NonPooled-4 7.101µ 7.297µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 6.484µ 6.625µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 9.455µ 10.025µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 9.352µ 9.712µ ~ 0.100
_prepareTxsPerLevel-4 406.1m 402.7m ~ 0.100
_prepareTxsPerLevelOrdered-4 3.734m 3.838m ~ 0.700
_prepareTxsPerLevel_Comparison/Original-4 417.5m 412.3m ~ 0.200
_prepareTxsPerLevel_Comparison/Optimized-4 3.581m 3.487m ~ 0.100
SubtreeProcessor/100_tx_64_per_subtree-4 82.34m 83.83m ~ 0.100
SubtreeProcessor/500_tx_64_per_subtree-4 399.7m 400.0m ~ 1.000
SubtreeProcessor/500_tx_256_per_subtree-4 412.2m 411.3m ~ 1.000
SubtreeProcessor/1k_tx_64_per_subtree-4 793.6m 794.3m ~ 1.000
SubtreeProcessor/1k_tx_256_per_subtree-4 816.3m 814.5m ~ 0.700
StreamingProcessorPhases/FilterValidated/100_tx-4 2.898m 2.890m ~ 1.000
StreamingProcessorPhases/ClassifyProcess/100_tx-4 246.9µ 246.7µ ~ 1.000
StreamingProcessorPhases/FilterValidated/500_tx-4 14.00m 13.81m ~ 0.700
StreamingProcessorPhases/ClassifyProcess/500_tx-4 600.6µ 595.8µ ~ 0.700
StreamingProcessorPhases/FilterValidated/1k_tx-4 27.62m 27.76m ~ 0.100
StreamingProcessorPhases/ClassifyProcess/1k_tx-4 1.055m 1.057m ~ 0.700
SubtreeSizes/10k_tx_4_per_subtree-4 1.345m 1.331m ~ 0.700
SubtreeSizes/10k_tx_16_per_subtree-4 322.4µ 326.8µ ~ 0.700
SubtreeSizes/10k_tx_64_per_subtree-4 77.25µ 77.93µ ~ 1.000
SubtreeSizes/10k_tx_256_per_subtree-4 19.19µ 19.42µ ~ 0.100
SubtreeSizes/10k_tx_512_per_subtree-4 9.649µ 9.642µ ~ 1.000
SubtreeSizes/10k_tx_1024_per_subtree-4 4.792µ 4.812µ ~ 0.400
SubtreeSizes/10k_tx_2k_per_subtree-4 2.367µ 2.399µ ~ 0.200
BlockSizeScaling/10k_tx_64_per_subtree-4 76.61µ 75.14µ ~ 1.000
BlockSizeScaling/10k_tx_256_per_subtree-4 19.12µ 19.34µ ~ 0.100
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.770µ 4.775µ ~ 1.000
BlockSizeScaling/50k_tx_64_per_subtree-4 398.7µ 403.6µ ~ 0.400
BlockSizeScaling/50k_tx_256_per_subtree-4 95.21µ 96.41µ ~ 0.200
BlockSizeScaling/50k_tx_1024_per_subtree-4 23.75µ 23.55µ ~ 0.700
SubtreeAllocations/small_subtrees_exists_check-4 157.9µ 160.7µ ~ 0.100
SubtreeAllocations/small_subtrees_data_fetch-4 166.6µ 168.2µ ~ 0.100
SubtreeAllocations/small_subtrees_full_validation-4 329.1µ 330.8µ ~ 0.400
SubtreeAllocations/medium_subtrees_exists_check-4 9.400µ 9.488µ ~ 0.700
SubtreeAllocations/medium_subtrees_data_fetch-4 9.881µ 9.784µ ~ 0.700
SubtreeAllocations/medium_subtrees_full_validation-4 19.23µ 18.92µ ~ 0.700
SubtreeAllocations/large_subtrees_exists_check-4 2.263µ 2.234µ ~ 0.700
SubtreeAllocations/large_subtrees_data_fetch-4 2.383µ 2.371µ ~ 0.400
SubtreeAllocations/large_subtrees_full_validation-4 4.780µ 4.783µ ~ 0.700
GetUtxoHashes-4 253.4n 256.7n ~ 0.700
GetUtxoHashes_ManyOutputs-4 43.53µ 42.40µ ~ 0.100
_NewMetaDataFromBytes-4 237.7n 237.7n ~ 1.000
_Bytes-4 628.8n 632.4n ~ 0.100
_MetaBytes-4 570.4n 574.2n ~ 0.100

Threshold: >10% with p < 0.05 | Generated: 2026-03-30 13:10 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sugh01