Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

col-cache: optimize for performance#1489

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
alubbe merged 1 commit intoexceljs:masterfrommyfreeer:col-cache-performance
Oct 5, 2020

Conversation

@myfreeer
Copy link
Contributor

Summary

Doing a profiling in chrome dev tools shows that the functiondecodeAddress andvalidateAddress inlib/utils/col-cache.js are taking considerable amount of time, mainly on the cache lookup and regexp matching. This would make that faster, especally with large worksheets.

Test plan

Existing unit test andnpm run benchmark result below.

Env: Windows 10 x64, node.js 12.18.4 LTS x64

npm run benchmark on master(a682091)
> exceljs@4.1.1 benchmark D:\UserData\projects\exceljs> node --expose-gc benchmark####################################################WARMUP: Current memory usage: 8.89 MBWARMUP: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsWARMUP: huge xlsx file streams profiling finished in 8354msWARMUP: Current memory usage (before GC): 153.16 MBWARMUP: Current memory usage (after GC): 41.63 MB####################################################RUN 1: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 1: huge xlsx file streams profiling finished in 8174msRUN 1: Current memory usage (before GC): 116.57 MBRUN 1: Current memory usage (after GC): 25.67 MB####################################################RUN 2: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 2: huge xlsx file streams profiling finished in 8048msRUN 2: Current memory usage (before GC): 142.09 MBRUN 2: Current memory usage (after GC): 41.6 MB####################################################RUN 3: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 3: huge xlsx file streams profiling finished in 8242msRUN 3: Current memory usage (before GC): 115.43 MBRUN 3: Current memory usage (after GC): 25.67 MB####################################################WARMUP: Current memory usage: 25.33 MBWARMUP: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsWARMUP: huge xlsx file async iteration profiling finished in 8120msWARMUP: Current memory usage (before GC): 148.28 MBWARMUP: Current memory usage (after GC): 41.65 MB####################################################RUN 1: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 1: huge xlsx file async iteration profiling finished in 8181msRUN 1: Current memory usage (before GC): 110.38 MBRUN 1: Current memory usage (after GC): 25.67 MB####################################################RUN 2: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 2: huge xlsx file async iteration profiling finished in 8215msRUN 2: Current memory usage (before GC): 148.74 MBRUN 2: Current memory usage (after GC): 41.64 MB####################################################RUN 3: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 3: huge xlsx file async iteration profiling finished in 8259msRUN 3: Current memory usage (before GC): 112.38 MBRUN 3: Current memory usage (after GC): 25.68 MB
npm run benchmark on myfreeer:col-cache-performance(8a47ae7)
> exceljs@4.1.1 benchmark D:\UserData\projects\exceljs> node --expose-gc benchmark####################################################WARMUP: Current memory usage: 8.89 MBWARMUP: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsWARMUP: huge xlsx file streams profiling finished in 6030msWARMUP: Current memory usage (before GC): 67.6 MBWARMUP: Current memory usage (after GC): 9.65 MB####################################################RUN 1: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 1: huge xlsx file streams profiling finished in 5787msRUN 1: Current memory usage (before GC): 53.34 MBRUN 1: Current memory usage (after GC): 9.75 MB####################################################RUN 2: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 2: huge xlsx file streams profiling finished in 5926msRUN 2: Current memory usage (before GC): 41.67 MBRUN 2: Current memory usage (after GC): 9.76 MB####################################################RUN 3: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 3: huge xlsx file streams profiling finished in 5808msRUN 3: Current memory usage (before GC): 40.32 MBRUN 3: Current memory usage (after GC): 9.76 MB####################################################WARMUP: Current memory usage: 9.47 MBWARMUP: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsWARMUP: huge xlsx file async iteration profiling finished in 5808msWARMUP: Current memory usage (before GC): 48.61 MBWARMUP: Current memory usage (after GC): 9.77 MB####################################################RUN 1: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 1: huge xlsx file async iteration profiling finished in 5837msRUN 1: Current memory usage (before GC): 38.44 MBRUN 1: Current memory usage (after GC): 9.77 MB####################################################RUN 2: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 2: huge xlsx file async iteration profiling finished in 5800msRUN 2: Current memory usage (before GC): 51.21 MBRUN 2: Current memory usage (after GC): 9.85 MB####################################################RUN 3: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 3: huge xlsx file async iteration profiling finished in 5793msRUN 3: Current memory usage (before GC): 48.64 MBRUN 3: Current memory usage (after GC): 9.83 MB

// check if value looks like an address
validateAddress(value){
if(!value.match(/^[A-Z]+\d+$/)){
// if (!value.match(/^[A-Z]+\d+$/))
Copy link
Member

@alubbealubbeOct 5, 2020•
edited
Loading

Choose a reason for hiding this comment

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

could you please benchmark if your new implementation is faster than this:

constaddressRegex=/^[A-Z]+\d+$/;// ...validateAddress(value){if(!addressRegex.test(value)){thrownewError(`Invalid Address:${value}`);}returntrue;}

This one would be a lot easier to read and maintain :D

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

npm run benchmark shows thataddressRegex.test is slightly slower, I'll switch to this if you wish.

npm run benchmark on myfreeer:col-cache-performance(8a47ae7) with validateAddress implementation mentioned above
> exceljs@4.1.1 benchmark D:\UserData\projects\exceljs> node --expose-gc benchmark####################################################WARMUP: Current memory usage: 8.87 MBWARMUP: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsWARMUP: huge xlsx file streams profiling finished in 6090msWARMUP: Current memory usage (before GC): 46.75 MBWARMUP: Current memory usage (after GC): 9.6 MB####################################################RUN 1: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 1: huge xlsx file streams profiling finished in 6063msRUN 1: Current memory usage (before GC): 44.24 MBRUN 1: Current memory usage (after GC): 9.75 MB####################################################RUN 2: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 2: huge xlsx file streams profiling finished in 5970msRUN 2: Current memory usage (before GC): 67.58 MBRUN 2: Current memory usage (after GC): 9.74 MB####################################################RUN 3: huge xlsx file streams profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 3: huge xlsx file streams profiling finished in 6220msRUN 3: Current memory usage (before GC): 69.27 MBRUN 3: Current memory usage (after GC): 9.79 MB####################################################WARMUP: Current memory usage: 9.47 MBWARMUP: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsWARMUP: huge xlsx file async iteration profiling finished in 6015msWARMUP: Current memory usage (before GC): 40.56 MBWARMUP: Current memory usage (after GC): 9.83 MB####################################################RUN 1: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 1: huge xlsx file async iteration profiling finished in 5956msRUN 1: Current memory usage (before GC): 37.71 MBRUN 1: Current memory usage (after GC): 9.82 MB####################################################RUN 2: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 2: huge xlsx file async iteration profiling finished in 5870msRUN 2: Current memory usage (before GC): 37.96 MBRUN 2: Current memory usage (after GC): 9.85 MB####################################################RUN 3: huge xlsx file async iteration profiling startedReading worksheet 1Reading row 50000Reading row 100000Reading worksheet 2Reading row 150000Processed 2 worksheets and 150002 rowsRUN 3: huge xlsx file async iteration profiling finished in 5902msRUN 3: Current memory usage (before GC): 39.57 MBRUN 3: Current memory usage (after GC): 9.88 MB

Copy link
Member

Choose a reason for hiding this comment

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

yeah let's do it - other than that, the PR looks great!

Doing a profiling in chrome dev tools shows that the function decodeAddress and validateAddress in lib/utils/col-cache.js are taking considerable amount of time, mainly on the cache lookup and regexp matching. This would make that faster, especially with large worksheets.
Copy link
Member

@alubbealubbe left a comment

Choose a reason for hiding this comment

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

thank you!

@alubbealubbe merged commit624d72f intoexceljs:masterOct 5, 2020
@myfreeermyfreeer deleted the col-cache-performance branchOctober 5, 2020 14:32
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@alubbealubbealubbe approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@myfreeer@alubbe
[8]ページ先頭
©2009-2025 Movatter.jp