Skip to content

perf(chunk): Optimized the combineChunk function - #27

Merged
nullure merged 1 commit into
CaviraOSS:mainfrom
DKB0512:dkb/optimize-combine-chunks
Oct 31, 2025
Merged

nullure merged 1 commit into
CaviraOSS:mainfrom
DKB0512:dkb/optimize-combine-chunks

Conversation

@DKB0512

@DKB0512 DKB0512 commented Oct 30, 2025

Copy link
Copy Markdown
Contributor

📋 Description

🔄 Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Style/UI changes
  • ♻️ Code refactoring
  • ⚡ Performance improvements
  • 🧪 Test updates
  • 🔧 Build/CI changes

🧪 Testing

  • I have tested this change locally
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

🔍 Code Review Checklist

  • Code follows the project's coding standards
  • Self-review of the code has been performed
  • Code is properly commented, particularly in hard-to-understand areas
  • Changes generate no new warnings
  • Any dependent changes have been merged and published

📋 Additional Context

.map() creates a new intermediate array of strings which causes extra memory allocation and garbage collection pressure. I've also compared the optimized function with .join(). The optimized version with a only for loop got 2x performance improvement (for medium-large dataset)
You can find that benchmark code here

Size    Original        Optimized       OptimizedWithJoin
0       0.0004ms        0.0001ms        0.0001ms
1       0.0018ms        0.0003ms        0.0004ms
5       0.0010ms        0.0017ms        0.0006ms
10      0.0015ms        0.0010ms        0.0013ms
50      0.0065ms        0.0032ms        0.0054ms
100     0.0176ms        0.0064ms        0.0098ms
500     0.0602ms        0.0204ms        0.0356ms
1000    0.1009ms        0.0414ms        0.0678ms
5000    0.4457ms        0.2139ms        0.3239ms
10000   0.8415ms        0.3765ms        0.5595ms

@nullure nullure left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@nullure
nullure merged commit 135db1a into CaviraOSS:main Oct 31, 2025
1 check passed
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.

2 participants