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

Read file as Buffer to mitigate V8's 256MB max string size #6

Open
JamesMGreene opened this issue Aug 10, 2017 · 5 comments
Open

Read file as Buffer to mitigate V8's 256MB max string size #6

JamesMGreene opened this issue Aug 10, 2017 · 5 comments
Assignees
Milestone

Comments

@JamesMGreene
Copy link
Owner

JamesMGreene commented Aug 10, 2017

Read file as Buffer, read lines as strings.

Implementation concept can be taken from existing NeDB PR: louischatriot/nedb#493

However, it's not a terribly performant implementation since it is doing things like concatenating an array with itself rather than just pushing new items on to it, so a rewrite is in order.

This would mitigate frustrating issues like the following:

@JamesMGreene
Copy link
Owner Author

This looks like a better implementation to consider: louischatriot/nedb#463

@JamesMGreene
Copy link
Owner Author

Seems like a symmetrical method for writing as a stream would also be necessary for the same reason.

@JamesMGreene
Copy link
Owner Author

JamesMGreene commented Aug 15, 2017

The default writeAsStream implementation should be very mindful of honoring backpressure cues from Node like in the examples from the documentation and other sources, e.g. this [imperfect] StackOverflow thread.

@JamesMGreene
Copy link
Owner Author

JamesMGreene commented Aug 24, 2017

IMPORTANT NOTE FOR BROWSER COMPATIBILITY

All Stream-specific module usage should be limited to "storage.js" (which is subbed out for the browser version, for example) and NOT put into "persistence.js" (which is NOT subbed out).

It is acceptable to have stream-related work in "persistence.js", e.g. listening for events, so long as it

  • is not required (i.e. still support the non-streaming version)
  • does not require additional module dependencies specific to streams

@JamesMGreene
Copy link
Owner Author

For reference: nodejs/node#3175

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant