-
-
Notifications
You must be signed in to change notification settings - Fork 48
perf!: allow reducing data retention, by adding retainSamples option on bench and task level, default is false
#421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reduces memory usage by replacing full sample arrays with sample counts in the Statistics interface, addressing issue #61 related to data retention.
Key Changes:
- Modified
Statisticsinterface to store onlysamplesCount: numberinstead ofsamples: number[] - Updated
getStatisticsSorted()to return sample count instead of the full array - Updated all tests and console table formatting to use
samplesCount
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Changed Statistics interface to replace samples array with samplesCount number |
| src/utils.ts | Updated getStatisticsSorted() to return samplesCount: samples.length and console table converter to use new property |
| src/task.ts | Added type annotation for throughputSamples and GC-related assignments (though the latter are not functionally necessary) |
| test/statistics.test.ts | Updated test assertions to check samplesCount type instead of array validation |
| test/concurrency-iteration-limit.test.ts | Updated test assertions to check samplesCount value instead of array length |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
retainSamples option on bench and task level
retainSamples option on bench and task levelretainSamples option on bench and task level, default is no samples retention
|
I made the data retention now configurable Do you have arguments against no data retention by default? |
retainSamples option on bench and task level, default is no samples retentionretainSamples option on bench and task level, default is samples retention
retainSamples option on bench and task level, default is samples retentionretainSamples option on bench and task level, default is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7bbb272 to
ea031f3
Compare
retainSamples option on bench and task level, default is trueretainSamples option on bench and task level, default is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 52 out of 53 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
retainSamples option on bench and task level, default is trueretainSamples option on bench and task level, default is false
fixes #61