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

Make Blob store an Arc<Vec<u8>> #8801

Closed
jdm opened this issue Dec 3, 2015 · 4 comments
Closed

Make Blob store an Arc<Vec<u8>> #8801

jdm opened this issue Dec 3, 2015 · 4 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Dec 3, 2015

The point of Blob values is that they're immutable buffers, and sometimes they're even slices of immutable buffers, and they're often used as sources of data that will be sent between threads internally. We should support this properly by making the underlying buffer threadsafe, allowing us to pass around references to it rather than cloning it. This will allow us to:

  • remove the read_out_buffer method, and rename clone_bytes to get_bytes
  • make Blob slices (ie. the result of calling Blob::Slice) share the original buffer, by storing the offset and length of the slice in the new Blob object

Code: components/script/dom/blob.rs

@craftytrickster
Copy link
Contributor

@craftytrickster craftytrickster commented Dec 5, 2015

I'd like to take this one.

@jdm jdm added the C-assigned label Dec 5, 2015
@jdm
Copy link
Member Author

@jdm jdm commented Dec 5, 2015

Go for it!

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Dec 6, 2015

Should this be Arc<Vec<u8>> or Arc<[u8]>?

@jdm
Copy link
Member Author

@jdm jdm commented Dec 6, 2015

I'm not sure how to create the Arc<[u8]> value from the various Blob constructors, so I'm going to go with Arc<Vec<u8>> for the purposes of this issue.

bors-servo added a commit that referenced this issue Jan 7, 2016
Changing blob to use arc pointers in order to limit wasteful copying …

Fixes #8801.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8860)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.