Skip to content

refactor: sampling optimalizations - #5167

Closed
nugaon wants to merge 11 commits into
masterfrom
feat/sampling-opts
Closed

refactor: sampling optimalizations#5167
nugaon wants to merge 11 commits into
masterfrom
feat/sampling-opts

Conversation

@nugaon

@nugaon nugaon commented Jul 23, 2025

Copy link
Copy Markdown
Member

This PR optimizes the ReserveSample function to improve performance during sampling operations. The key optimizations include adding ChunkType to the SampleItem struct to avoid redundant parsing, reusing hashers in worker goroutines instead of creating new ones for each SOC chunk and using new optimized hasher function for transformed address calculations. These changes reduce memory allocations, GC pressure, and CPU usage without altering the functional behavior of the sampling process.

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

@nugaon nugaon changed the title feat: sampling opts refactor: sampling optimalizations Jul 24, 2025
@nugaon
nugaon marked this pull request as ready for review July 24, 2025 13:59
Comment thread pkg/storer/sample.go
}()

sampleItems := make([]SampleItem, 0, SampleSize)
sampleItems := make([]SampleItem, 0, SampleSize+1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why +1 ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower there is a condition when if len(sampleItems) > SampleSize it cuts from the end. so sampleItems used length is SampleSize+1.

@nugaon nugaon linked an issue Jul 29, 2025 that may be closed by this pull request
@martinconic
martinconic requested review from janos and removed request for acha-bill August 20, 2025 13:44
@sbackend123
sbackend123 self-requested a review September 10, 2025 08:09
Comment thread pkg/swarm/hasher.go Outdated
Comment thread pkg/swarm/hasher.go Outdated
Comment thread pkg/swarm/hasher.go

// NewPrefixHasher returns new hasher which is Keccak-256 hasher
// with prefix value added as initial data.
func NewPrefixHasher(prefix []byte) hash.Hash {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also called in proof.go. Do we want that one to use the stateful hasher?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is used in the sample creation as well and both should do the same calculation.

Comment thread pkg/swarm/hasher.go
}

// NewHasher returns new Keccak-256 hasher.
func NewStatefulHasher(state []byte) (sha3.StatefulHash, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should new tests be added in hasher_test for this new stateful hasher?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nugaon
nugaon requested a review from martinconic September 12, 2025 20:24
@acud

acud commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

closing this as we are cleaning up the open pull requests, and other changes are inbound for this topic. if there are any improvements here, we will implement them in subsequent PRs.

@acud acud closed this Mar 19, 2026
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.

Optimize ReserveSample Function

3 participants