Skip to content

string_decoder: optimize write()#1209

Closed
mscdex wants to merge 1 commit intonodejs:v1.xfrom
mscdex:perf-stringdecoder
Closed

string_decoder: optimize write()#1209
mscdex wants to merge 1 commit intonodejs:v1.xfrom
mscdex:perf-stringdecoder

Conversation

@mscdex
Copy link
Copy Markdown
Contributor

@mscdex mscdex commented Mar 19, 2015

By limiting property getting/setting to only where they are
absolutely necessary, we can achieve greater performance
especially with small utf8 inputs and any size base64 inputs.

ascii/binary are unaffected.

Example results for utf8 and base64 from the included benchmark:

misc/string-decode.js encoding=utf8 inlen=32 chunk=16 n=250000: ./iojs: 1946800 ./iojs-io.js-4655521: 1654400 ........... 17.67%
misc/string-decode.js encoding=utf8 inlen=32 chunk=64 n=250000: ./iojs: 2838500 ./iojs-io.js-4655521: 2545000 ........... 11.53%
misc/string-decode.js encoding=utf8 inlen=32 chunk=256 n=250000: ./iojs: 2822200 ./iojs-io.js-4655521: 2522400 .......... 11.88%
misc/string-decode.js encoding=utf8 inlen=32 chunk=1024 n=250000: ./iojs: 2829100 ./iojs-io.js-4655521: 2507800 ......... 12.81%
misc/string-decode.js encoding=utf8 inlen=128 chunk=16 n=250000: ./iojs: 508720 ./iojs-io.js-4655521: 499830 ............. 1.78%
misc/string-decode.js encoding=utf8 inlen=128 chunk=64 n=250000: ./iojs: 1016400 ./iojs-io.js-4655521: 931340 ............ 9.13%
misc/string-decode.js encoding=utf8 inlen=128 chunk=256 n=250000: ./iojs: 1250200 ./iojs-io.js-4655521: 1179200 .......... 6.02%
misc/string-decode.js encoding=utf8 inlen=128 chunk=1024 n=250000: ./iojs: 1258600 ./iojs-io.js-4655521: 1193400 ......... 5.46%
misc/string-decode.js encoding=utf8 inlen=1024 chunk=16 n=250000: ./iojs: 64944 ./iojs-io.js-4655521: 64852 .............. 0.14%
misc/string-decode.js encoding=utf8 inlen=1024 chunk=64 n=250000: ./iojs: 134380 ./iojs-io.js-4655521: 133250 ............ 0.85%
misc/string-decode.js encoding=utf8 inlen=1024 chunk=256 n=250000: ./iojs: 178810 ./iojs-io.js-4655521: 176190 ........... 1.49%
misc/string-decode.js encoding=utf8 inlen=1024 chunk=1024 n=250000: ./iojs: 215380 ./iojs-io.js-4655521: 212140 .......... 1.52%
misc/string-decode.js encoding=base64-utf8 inlen=32 chunk=16 n=250000: ./iojs: 407580 ./iojs-io.js-4655521: 307650 ...... 32.48%
misc/string-decode.js encoding=base64-utf8 inlen=32 chunk=64 n=250000: ./iojs: 1422000 ./iojs-io.js-4655521: 1120100 .... 26.96%
misc/string-decode.js encoding=base64-utf8 inlen=32 chunk=256 n=250000: ./iojs: 1416700 ./iojs-io.js-4655521: 1160400 ... 22.09%
misc/string-decode.js encoding=base64-utf8 inlen=32 chunk=1024 n=250000: ./iojs: 1339300 ./iojs-io.js-4655521: 1140600 .. 17.42%
misc/string-decode.js encoding=base64-utf8 inlen=128 chunk=16 n=250000: ./iojs: 93840 ./iojs-io.js-4655521: 84048 ....... 11.65%
misc/string-decode.js encoding=base64-utf8 inlen=128 chunk=64 n=250000: ./iojs: 383150 ./iojs-io.js-4655521: 308310 ..... 24.28%
misc/string-decode.js encoding=base64-utf8 inlen=128 chunk=256 n=250000: ./iojs: 1067600 ./iojs-io.js-4655521: 886690 ... 20.41%
misc/string-decode.js encoding=base64-utf8 inlen=128 chunk=1024 n=250000: ./iojs: 1011700 ./iojs-io.js-4655521: 926400 ... 9.21%
misc/string-decode.js encoding=base64-utf8 inlen=1024 chunk=16 n=250000: ./iojs: 13682 ./iojs-io.js-4655521: 10591 ...... 29.19%
misc/string-decode.js encoding=base64-utf8 inlen=1024 chunk=64 n=250000: ./iojs: 48976 ./iojs-io.js-4655521: 39895 ...... 22.76%
misc/string-decode.js encoding=base64-utf8 inlen=1024 chunk=256 n=250000: ./iojs: 141090 ./iojs-io.js-4655521: 120230 ... 17.35%
misc/string-decode.js encoding=base64-utf8 inlen=1024 chunk=1024 n=250000: ./iojs: 269880 ./iojs-io.js-4655521: 244070 .. 10.57%
misc/string-decode.js encoding=base64-ascii inlen=32 chunk=16 n=250000: ./iojs: 497550 ./iojs-io.js-4655521: 396600 ..... 25.45%
misc/string-decode.js encoding=base64-ascii inlen=32 chunk=64 n=250000: ./iojs: 1521700 ./iojs-io.js-4655521: 1174800 ... 29.52%
misc/string-decode.js encoding=base64-ascii inlen=32 chunk=256 n=250000: ./iojs: 1487500 ./iojs-io.js-4655521: 1174100 .. 26.69%
misc/string-decode.js encoding=base64-ascii inlen=32 chunk=1024 n=250000: ./iojs: 1401600 ./iojs-io.js-4655521: 1155800 . 21.27%
misc/string-decode.js encoding=base64-ascii inlen=128 chunk=16 n=250000: ./iojs: 142030 ./iojs-io.js-4655521: 114770 .... 23.75%
misc/string-decode.js encoding=base64-ascii inlen=128 chunk=64 n=250000: ./iojs: 499170 ./iojs-io.js-4655521: 396290 .... 25.96%
misc/string-decode.js encoding=base64-ascii inlen=128 chunk=256 n=250000: ./iojs: 1145200 ./iojs-io.js-4655521: 999490 .. 14.57%
misc/string-decode.js encoding=base64-ascii inlen=128 chunk=1024 n=250000: ./iojs: 1207200 ./iojs-io.js-4655521: 983240 . 22.77%
misc/string-decode.js encoding=base64-ascii inlen=1024 chunk=16 n=250000: ./iojs: 18163 ./iojs-io.js-4655521: 14729 ..... 23.31%
misc/string-decode.js encoding=base64-ascii inlen=1024 chunk=64 n=250000: ./iojs: 69888 ./iojs-io.js-4655521: 60726 ..... 15.09%
misc/string-decode.js encoding=base64-ascii inlen=1024 chunk=256 n=250000: ./iojs: 194780 ./iojs-io.js-4655521: 141810 .. 37.36%
misc/string-decode.js encoding=base64-ascii inlen=1024 chunk=1024 n=250000: ./iojs: 356430 ./iojs-io.js-4655521: 342270 .. 4.14%

@mscdex
string_decoder: optimize write()
6d815b7 
By limiting property getting/setting to only where they are
absolutely necessary, we can achieve greater performance
especially with small utf8 inputs and any size base64 inputs.
@rvagg
Copy link
Copy Markdown
Member

rvagg commented Mar 19, 2015

nice, I like those utf8 numbers, any idea how much property getting is impacting this?

LGTM

@mscdex
Copy link
Copy Markdown
Contributor Author

mscdex commented Mar 19, 2015

I made these changes awhile ago so I can't recall for sure, but this patch is almost entirely made up of changes to avoid property lookups, so I would say that it is impacting things the most (especially in the loop in write()) versus some of the other minor non-property-related changes.

@mscdex
Copy link
Copy Markdown
Contributor Author

mscdex commented Mar 20, 2015

/cc @iojs/collaborators

@micnic
Copy link
Copy Markdown
Contributor

micnic commented Mar 20, 2015

LGTM

@petkaantonov
Copy link
Copy Markdown
Contributor

petkaantonov commented Mar 20, 2015

This should be implemented in C, we will need utf8 decoder in the future for buffer.toString() which is currently delegating to V8's utf8 decoder.

@mscdex
Copy link
Copy Markdown
Contributor Author

mscdex commented Mar 20, 2015

@petkaantonov Why? Is V8 dropping their UTF-8 decoder soon?

@petkaantonov
Copy link
Copy Markdown
Contributor

petkaantonov commented Mar 20, 2015

@mscdex No but instead of using NewFromUtf8 (which v8 internally decodes and copies to jsheap), we would decode it ourselves into an external buffer which doesn't take space in jsheap and also allows string sizes bigger than 290mb or so. See #649

chrisdickinson
chrisdickinson reviewed Mar 20, 2015
View reviewed changes
Comment thread lib/string_decoder.js
Copy link
Copy Markdown
Contributor

@chrisdickinson chrisdickinson Mar 20, 2015

Choose a reason for hiding this comment

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

I wonder if V8 will unroll the subsequent loop (or if there's value in doing so ourselves.)

Copy link
Copy Markdown
Contributor Author

@mscdex mscdex Mar 20, 2015

Choose a reason for hiding this comment

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

You mean for the i = 3 case? I didn't try that since I figured people would frown about code duplication between the i = 3 and i = buflen cases. I also did not see what kind performance difference it would make though.

@chrisdickinson
Copy link
Copy Markdown
Contributor

chrisdickinson commented Mar 20, 2015

This LGTM.

This should be implemented in C, we will need utf8 decoder in the future for buffer.toString() which is currently delegating to V8's utf8 decoder.

We'll still have to support this module if we move to a C decoder, so making it faster is probably worthwhile.

@mscdex mscdex added the string_decoder Issues and PRs related to the string_decoder subsystem. label Mar 25, 2015
mscdex added a commit that referenced this pull request Mar 25, 2015
@mscdex
string_decoder: optimize write()
8a94581 
By limiting property getting/setting to only where they are
absolutely necessary, we can achieve greater performance
especially with small utf8 inputs and any size base64 inputs.

PR-URL: #1209
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Nicu Micleușanu <[email protected]>
Reviewed-By: Chris Dickinson <[email protected]>
@mscdex
Copy link
Copy Markdown
Contributor Author

mscdex commented Mar 25, 2015

Landed in 8a94581.

@mscdex mscdex closed this Mar 25, 2015
@mscdex mscdex deleted the perf-stringdecoder branch March 25, 2015 04:39
@rvagg rvagg mentioned this pull request Mar 28, 2015
Closed
@Fishrock123 Fishrock123 mentioned this pull request Apr 9, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

string_decoder Issues and PRs related to the string_decoder subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mscdex @rvagg @micnic @petkaantonov @chrisdickinson