Open benatkin opened 1 year ago
Hi @benatkin,
Can you show some kind of benchmarks that readAsDataURL outperforms the current approach?
In any case, the current implementation is fast enough as operations on typed arrays are quickly optimized by JavaScript runtimes. They perform almost as well as C code.
Sorry for the delayed update. I did one with Deno bench. I probably need to make it vary the arrays, but I don't think it would cache the answers. It says the FileReader one is quite a bit faster:
Last login: Sun Dec 4 23:40:15 on ttys050
~ ❯❯❯
~ ❯❯❯ cd projects/resources/misc
~/p/r/misc ❯❯❯ deno bench bench-base64.js
~/p/r/misc ❯❯❯ deno bench bench-base64.js
cpu: Apple M1
runtime: deno 1.28.3 (aarch64-apple-darwin)
file:///Users/bat/projects/resources/misc/bench-base64.js
benchmark time (avg) (min … max) p75 p99 p995
---------------------------------------------------------- -----------------------------
with FileReader 56.74 µs/iter (32.29 µs … 2.16 ms) 49.29 µs 609.79 µs 804.71 µs
without FileReader 795.87 µs/iter (737.04 µs … 1.43 ms) 760.29 µs 1.33 ms 1.36 ms
~/p/r/misc ❯❯❯ cat bench-base64.js
const toBase64Browser = bytes => {
let s = ''
for (let i = 0; i < bytes.byteLength; i++) {
s += String.fromCharCode(bytes[i])
}
// eslint-disable-next-line no-undef
return btoa(s)
}
function readAsync(file) {
return new Promise((resolve, reject) => {
const reader = new FileReader();
reader.onload = () => resolve(reader.result)
reader.onerror = reject;
reader.readAsDataURL(file);
})
}
const toBase64DataUrl = async bytes => {
const s = await readAsync(new Blob([bytes]))
return s.substring(s.indexOf(',') + 1)
}
const byteArray = new Uint8Array(50000)
const s1 = toBase64Browser(byteArray)
const s2 = await toBase64DataUrl(byteArray)
if (s1 !== s2) {
throw new Error(`Does not match: s1 (${s1.length}) and s2 (${s2.length})`)
}
Deno.bench('with FileReader', async () => {
await toBase64DataUrl(byteArray)
})
Deno.bench('without FileReader', async () => {
toBase64Browser(byteArray)
})
~/p/r/misc ❯❯❯
This was converting to base64, I omitted converting from base64 - I agree it doesn't seem like it should be faster since it's a native code to native code comparison. I think the string concatenation still slows code down though, and that is used in converting to base64.
Is your feature request related to a problem? Please describe.
When using an isometric library I prefer them to use functionality from the platform where possible. This uses fromCharCode to convert from binary to base64 when FileReader.readAsDataURL() ought to work.
Describe the solution you'd like
Wrapping FileReader.readAsDataURL would be preferable, because instead of looping through each byte in JS, a native method does the whole thing.
https://developer.mozilla.org/en-US/docs/Web/API/FileReader/readAsDataURL
Describe alternatives you've considered
I considered using platform functionality everywhere if it would work, however, FileReader.readAsDataURL is not available on Node. Uint8Array is on Node. So they still need separate ways to work.
Additional context
buffer.toBase64
is used in y-webrtc I was thinking of using it for saving Y Docs to localStorage but may end up using IndexedDB. My code already uses localStorage and IndexedDB isn't supported on Firefox Private Browsing sessions so that's why I may keep using localStorage for the time being.