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 IOCompositor only respond to the first Quit event. #5245

Merged
merged 1 commit into from Mar 17, 2015

Conversation

@nnethercote
Copy link
Contributor

nnethercote commented Mar 17, 2015

This fixes #5234, in that the huge memory spike disappears. It still takes ~15 seconds for the window to actually disappear after that first Quit event is received by the IOCompositor. Maybe that's a pre-existing problem.

There may be better ways to do this, like handling it on the sending side (i.e. within glutin) instead of the receiving side. I just did it this way because it seemed like the easiest thing.

This avoids huge mpsc_queue build-ups from the flood of Quit events
coming from glutin.

Fixes #5234.
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Mar 17, 2015

Critic review: https://critic.hoppipolla.co.uk/r/4280

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@jdm
Copy link
Member

jdm commented Mar 17, 2015

r? @glennw

@glennw

This comment has been minimized.

Copy link

glennw commented on 2b10f6e Mar 17, 2015

r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 2b10f6e Mar 17, 2015

saw approval from glennw
at nnethercote@2b10f6e

This comment has been minimized.

Copy link
Contributor

bors-servo replied Mar 17, 2015

merging nnethercote/servo/only-one-quit-event = 2b10f6e into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Mar 17, 2015

nnethercote/servo/only-one-quit-event = 2b10f6e merged ok, testing candidate = 2281bca

This comment has been minimized.

Copy link
Contributor

bors-servo replied Mar 17, 2015

fast-forwarding master to auto = 2281bca

bors-servo pushed a commit that referenced this pull request Mar 17, 2015
This fixes #5234, in that the huge memory spike disappears. It still takes ~15 seconds for the window to actually disappear after that first `Quit` event is received by the IOCompositor. Maybe that's a pre-existing problem.

There may be better ways to do this, like handling it on the sending side (i.e. within glutin) instead of the receiving side. I just did it this way because it seemed like the easiest thing.
@bors-servo bors-servo closed this Mar 17, 2015
@bors-servo bors-servo merged commit 2b10f6e into servo:master Mar 17, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default all tests passed
@nnethercote nnethercote deleted the nnethercote:only-one-quit-event branch Mar 24, 2015
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.

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