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 the example blob renderer multi-threaded. #1179

Merged
merged 1 commit into from May 17, 2017

Conversation

@nical
Copy link
Collaborator

nical commented Apr 28, 2017

Update the blob image renderer example to render all images in parallel using the (infamous) ThreadPool.


This change is Reviewable

@nical
Copy link
Collaborator Author

nical commented Apr 28, 2017

}
}

return Ok(RasterizedBlobImage {

This comment has been minimized.

@kvark

kvark Apr 28, 2017

Member

no need for return

// The deserialized drawing commands.
image_cmds: HashMap<ImageKey, ImageRenderingCommands>,
// We are going to defer the rendering work to worker threads.
// using a pre-built Arc<Mutex<ThreadPool>> rather than creating our own threads

This comment has been minimized.

@kvark

kvark Apr 28, 2017

Member

nit: Using

fn resolve(&mut self, request: BlobImageRequest) -> BlobImageResult {
self.rendered_images.remove(&request).unwrap_or(Err(BlobImageError::InvalidKey))
// if we break out of the loop above it means the channel closed unexpectedly.
return Err(BlobImageError::Other("Channel closed".into()));

This comment has been minimized.

@kvark

kvark Apr 28, 2017

Member

no return needed

@bors-servo
Copy link
Contributor

bors-servo commented May 3, 2017

The latest upstream changes (presumably #1181) made this pull request unmergeable. Please resolve the merge conflicts.

@glennw
Copy link
Member

glennw commented May 16, 2017

@nical Needs a rebase.

@nical
Copy link
Collaborator Author

nical commented May 16, 2017

Yep, I am waiting to see if the switch to rayon sticks before I update this PR for it.

@glennw
Copy link
Member

glennw commented May 16, 2017

@nical We landed the WR update with rayon in servo today - so it should be fine to update this :)

@nical nical force-pushed the nical:blob-threads branch from 9477d45 to 9082acb May 16, 2017
@glennw
Copy link
Member

glennw commented May 17, 2017

Seems to be some compile errors on CI.

@nical nical force-pushed the nical:blob-threads branch from 0622b65 to 0b90152 May 17, 2017
@kvark
Copy link
Member

kvark commented May 17, 2017

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2017

📌 Commit 0b90152 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2017

Testing commit 0b90152 with merge ff6bb6a...

bors-servo added a commit that referenced this pull request May 17, 2017
Make the example blob renderer multi-threaded.

Update the blob image renderer example to render all images in parallel using the (infamous) ThreadPool.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1179)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2017

💔 Test failed - status-travis

@kvark
Copy link
Member

kvark commented May 17, 2017

OSX timeout, retrying
@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2017

Testing commit 0b90152 with merge 4600023...

bors-servo added a commit that referenced this pull request May 17, 2017
Make the example blob renderer multi-threaded.

Update the blob image renderer example to render all images in parallel using the (infamous) ThreadPool.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1179)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2017

☀️ Test successful - status-travis
Approved by: kvark
Pushing 4600023 to master...

@bors-servo bors-servo merged commit 0b90152 into servo:master May 17, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.