NotificationsYou must be signed in to change notification settings
Fork1.9k
Star14.8k

Increase the performance of some xml and html helpers#1476

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

Jump to bottom

Merged

alubbe merged 2 commits intoexceljs:masterfromalubbe:performance

Sep 29, 2020

Merged

Increase the performance of some xml and html helpers#1476

alubbe merged 2 commits intoexceljs:masterfromalubbe:performance

Sep 29, 2020

Conversation

Copy link

Member

alubbe commentedSep 28, 2020

Summary

We saw some performance issues in#742 that this PR tries to solve

Test plan

Both helpers are already tested, this just refactors their implementation.

alubbe mentioned this pull request

Sep 28, 2020

Low Performance.#742

Closed

Increase the performance of some xml and html helpers

1ce9b52

alubbe force-pushed theperformance branch from2a99286 to1ce9b52Compare

September 28, 2020 10:13

alubbe requested review fromAlanscut,guyonroche andSiemienik

September 28, 2020 10:37

Siemienik approved these changes

Sep 28, 2020

View reviewed changes

Copy link

Member

Siemienik 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 for this PR, nice done
How big speedup you note after these changes?

rogozind reviewed

Sep 29, 2020

View reviewed changes

lib/utils/under-dash.js OutdatedShow resolvedHide resolved

Copy link

rogozind commentedSep 29, 2020

This is good. Regexp way of finding whether or not we need the escaping is even faster than the loop I suggested#742 .
In my test I got 25% increase in performance. I have 43K rows of data. It takes 20 secs to export. 50% used to go to create the spreadsheet and 50% used to go to write out the files. Out of these 10 secs 5 secs went to serialize various elements and 5 secs went to xmlEncode() calls. With the patch the xmlEncode dropped to near zero so export now takes 15secs.

Please accept this PR.