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

Split global constellation receiver into two (script and compositor) #8356

Closed
jdm opened this issue Nov 5, 2015 · 4 comments
Closed

Split global constellation receiver into two (script and compositor) #8356

jdm opened this issue Nov 5, 2015 · 4 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Nov 5, 2015

There are a couple parts to this:

  • replace the single receiver with two separate ones
    • make the constellation event loop (Constellation::run) select over the two receivers (see layout_task.rs for a similar model)
    • provide the appropriate sender when creating new script tasks (probably requires returning a second sender from Constellation::start)
  • split the constellation_msg::Msg enum into constellation_msg::ScriptMsg and constellation_msg::CompositorMsg enums that only contain the messages that are appropriate for each sender, and update the types of the various senders and receivers involved, and update all users of the newly-typed senders

Code: components/compositing/constellation.rs, components/msg/constellation_msg.rs

@jdm
Copy link
Member Author

@jdm jdm commented Nov 5, 2015

After this change it will be easier to tell if there are risks of deadlocks between the constellation and the script tasks, since there won't be a confusing mix of "does script actually use the message type" variants to consider.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Nov 9, 2015

We might want to add an additional constellation_msg::NetworkMsg for all the flavors of fetch as described by #4576.

I'll take a look at this issue over the weekend and see what I can do.

bors-servo added a commit that referenced this issue Nov 13, 2015
Split chan and receiver_port into script and compositor flavors

Partial #8356. Currently this doesn't build because of a lint denying me to user unsafe code, which unfortunately the select! macro falls under. Not sure what to do there.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8478)
<!-- Reviewable:end -->
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Nov 14, 2015

Quick question: will I run into type incompatibilities if I split constellation_msg::Msg into constellation_msg::ScriptMsg and constellation_msg::CompositorMsg while I am receiving messages from the channel? We currently now have the following:

    fn run(&mut self) {
        loop {
            let request = {
                let receiver_from_script = &self.script_receiver;
                let receiver_from_compositor = &self.compositor_receiver;
                select! {
                    msg = receiver_from_script.recv() => msg.unwrap(),
                    msg = receiver_from_compositor.recv() => msg.unwrap()
                }
            };
            if !self.handle_request(request) {
                break;
            }
        }
    }

and receiver_from_script has the type of Receiver<ScriptMsg> and receiver_from_compositor has the type of Receiver<CompositorMsg>.

@jdm
Copy link
Member Author

@jdm jdm commented Nov 14, 2015

Yep! This is why we we stuff like http://mxr.mozilla.org/servo/source/components/layout/layout_task.rs#482 in other places.

@jdm jdm added the C-assigned label Nov 18, 2015
bors-servo added a commit that referenced this issue Nov 18, 2015
Split ConstellationMsg into ScriptMsg and CompositorMsg

Fixes #8356.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8530)
<!-- 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
2 participants
You can’t perform that action at this time.