You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/10/17 00:43:43 UTC
[GitHub] [cordova-js] raphinesse opened a new pull request #242: perf(fromArrayBuffer): use less memory for large buffers
raphinesse opened a new pull request #242:
URL: https://github.com/apache/cordova-js/pull/242
### Platforms affected
All
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
This improves performance for converting large files to Base64 strings (see apache/cordova-plugin-file#364 for more info, especially the extensive performance analysis by @LightMind).
Another win here, is that we reduce the code size considerably. A downside is that we actually loose a bit of performance for small instances, but I do not think that this would be noticeable.
I explored different approaches here, like converting the original algorithm to use ArrayBuffers. The problem here is that we still need to convert the end result to a string which then becomes a performance bottleneck if you do not have support for `TextDecoder` (which we cannot assume with our current ES5 target).
Fixes #241
### Description
<!-- Describe your changes in detail -->
`base64.fromArrayBuffer` now uses `btoa` to convert bytes to a base64 encoded string. We already use its counterpart `atob` in `base64.toArrayBuffer`. Since `btoa` unfortunately operates on binary strings instead of buffers, we first need to convert the raw bytes to a [binary string](https://developer.mozilla.org/en-US/docs/Web/API/DOMString/Binary). This is the main performance bottleneck here, but applying `String.fromCharCode` to large chunks of data works reasonably well.
### Testing
<!-- Please describe in detail how you tested your changes. -->
I added a test that should hopefully preventing people from making stupid changes in the future. However, its reliance on an absolute expected runtime might lead to problems in the future.
I also did some performance comparisons (ops/s) between the new and old version in my local Chrome browser:
|bytes | old | new | new/old|
|------|-------|-------|--------|
| 1K | 61409 | 33029 | 54%|
| 32K | 1569 | 913 | 58%|
| 1M | 11 | 24 | 218%|
| 32M | .3 | .9 | 300%|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-js] raphinesse merged pull request #242: perf(fromArrayBuffer): use less memory for large buffers
Posted by GitBox <gi...@apache.org>.
raphinesse merged pull request #242:
URL: https://github.com/apache/cordova-js/pull/242
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-js] codecov-commenter edited a comment on pull request #242: perf(fromArrayBuffer): use less memory for large buffers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #242:
URL: https://github.com/apache/cordova-js/pull/242#issuecomment-944911591
# [Codecov](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#242](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (691ccc1) into [master](https://codecov.io/gh/apache/cordova-js/commit/af904b73633c6fa16e8cef0472fba785cb49e8f6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (af904b7) will **decrease** coverage by `0.57%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #242 +/- ##
==========================================
- Coverage 84.23% 83.65% -0.58%
==========================================
Files 14 14
Lines 539 520 -19
==========================================
- Hits 454 435 -19
Misses 85 85
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [src/common/base64.js](https://codecov.io/gh/apache/cordova-js/pull/242/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2NvbW1vbi9iYXNlNjQuanM=) | `100.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [af904b7...691ccc1](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-js] raphinesse commented on pull request #242: perf(fromArrayBuffer): use less memory for large buffers
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #242:
URL: https://github.com/apache/cordova-js/pull/242#issuecomment-944930287
For future reference: replacing `bytesToBinaryString(array)` with `new TextDecoder('utf-16').decode(Uint16Array.from(array))` doubles the throughput for large files in my experiments. In ten years we might be able to use it :wink:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-js] codecov-commenter commented on pull request #242: perf(fromArrayBuffer): use less memory for large buffers
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #242:
URL: https://github.com/apache/cordova-js/pull/242#issuecomment-944911591
# [Codecov](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#242](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (691ccc1) into [master](https://codecov.io/gh/apache/cordova-js/commit/af904b73633c6fa16e8cef0472fba785cb49e8f6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (af904b7) will **decrease** coverage by `0.63%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #242 +/- ##
==========================================
- Coverage 84.23% 83.59% -0.64%
==========================================
Files 14 12 -2
Lines 539 518 -21
==========================================
- Hits 454 433 -21
Misses 85 85
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [src/common/base64.js](https://codecov.io/gh/apache/cordova-js/pull/242/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2NvbW1vbi9iYXNlNjQuanM=) | `100.00% <100.00%> (ø)` | |
| [test/test-platform-modules/exec.js](https://codecov.io/gh/apache/cordova-js/pull/242/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdC90ZXN0LXBsYXRmb3JtLW1vZHVsZXMvZXhlYy5qcw==) | | |
| [test/test-platform-modules/platform.js](https://codecov.io/gh/apache/cordova-js/pull/242/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdC90ZXN0LXBsYXRmb3JtLW1vZHVsZXMvcGxhdGZvcm0uanM=) | | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [af904b7...691ccc1](https://codecov.io/gh/apache/cordova-js/pull/242?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org