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

Add a DynamicBuffer for faster VFS implementations #258

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

rkistner
Copy link
Contributor

Currently, the InMemoryFileSystem uses a Uint8List buffer per file. When resizing the buffer, it creates a new one and copies the content over.

While copying the bytes is fairly fast for small buffers, doing that for every written page for a 10MB+ file size adds significant overhead. Basically, creating a database with size n takes O(n^2) time, which becomes very noticeable with large databases. This affects both IndexedDbFileSystem and InMemoryFileSystem, since IndexedDbFileSystem uses InMemoryFileSystem internally. In some of my tests, creating a 30MB database takes around 120s with either VFS.

This introduces a DynamicBuffer class that handles resizing the buffer. Instead of resizing on every write, it doubles the size every time new capacity is needed. This changes it to O(n) to create a database of size n. In my tests, the same database takes 5s to create with InMemoryFileSystem, and 10s with IndexedDbFileSystem.

Another approach would be to use the JS ArrayBuffer.resize together with maxByteLength, since ArrayBuffers are used internally for ByteBuffer with dart2js anyway. However, it may not be supported by all browsers, and would need a different approach for dart2wasm.

In my testing this appears to work well with both IndexedDbFileSystem and InMemoryFileSystem. It would still be good to review the IndexedDbFileSystem implementation specifically - I'm not sure whether the change could introduce some edge case, especially around previousContent.

Copy link
Owner

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

Thanks! I agree with the better management for handling growing data.

If possible, I prefer using an Uint8Buffer from package:typed_data for this though. That class is supposed to be doing the same thing, it's a core Dart package and one less thing to maintain here :)

@rkistner
Copy link
Contributor Author

Great, I wasn't aware of that package - switched over now.

@simolus3 simolus3 merged commit a0032ff into simolus3:main Oct 18, 2024
7 of 13 checks passed
@simolus3
Copy link
Owner

Thanks! I've merged this and will upload a this weekend.

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