Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide ability to write as text instead of as Blob #35

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

TimHambourger
Copy link

Currently, write() writes everything as a Blob. On iOS and Android, cordova-plugin-file converts Blobs to ArrayBuffers, which the iOS/Android platforms then convert to base64 for transfer to the native layer. (Windows supports marshaling Blobs directly from JS to native, so that platform skips the ArrayBuffer and base64 conversions.)

I haven't profiled Android in as much detail, but on iOS that conversion to base64 is slow and memory intensive. (Some rough timings below.) An easy workaround for text files is to skip the conversion to Blob and write as text. That satisfies our use case of writing large JSON files.

What is the intention behind using Blobs? This PR currently comments out the Blob conversion completely. I'm assuming that breaks other use cases, so I'm open to amending the PR. For our purposes, we just need a way to opt out of the Blob conversion. An optional flag to write() would be great. I could also imagine a CordovaPromiseFS constructor option, or maybe the behavior should be platform dependent. I figured I'd start with the simple approach before reaching out about API design.

write() timings:
OS: iOS 11.3
Device: iPad Model A 1489 (iPad Mini 2nd Gen)
Cordova iOS platform: 4.4.0
cordova-plugin-file version: 4.3.3
cordova-promise-fs version: 1.2.5

Test - Write a 3.0MB JSON file. Use console.time to measure time from calling write() until Promise is resolved.
with Blob conversion - 3304 ms (median of 18 runs)
writing as text - 575 ms (median of 19 runs)

Let me know if I can provide more info.

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