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.

Sign up for GitHub

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

Jump to bottom

Lessen pressure on the thread-local RNG #12890

Merged
merged 1 commit into from Aug 16, 2016
Merged

Lessen pressure on the thread-local RNG #12890

merged 1 commit into from Aug 16, 2016

Conversation

nox
Copy link
Contributor

@nox nox commented Aug 16, 2016 •
edited by larsbergstrom

This change is Reviewable

@nox
Lessen pressure on the thread-local RNG
6cc862d 
When loading HTTP requests, initialise the request ID only if devtools
are used, to lessen the pressure put on the RNG initialisation.

This tentatively fixes servo#11100 on OS X.
@highfive
Copy link

Heads up! This PR modifies the following files:

  • @KiChjang: components/net/fetch/methods.rs, components/net/http_loader.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 16, 2016
@nox
Copy link
Contributor Author

nox commented Aug 16, 2016

r? @metajack

@highfive highfive assigned metajack and unassigned Manishearth Aug 16, 2016
@Manishearth
Copy link
Member

r=me unless you want to wait for jack

not fond of the unwrap, but I guess it's okay

@nox
Copy link
Contributor Author

nox commented Aug 16, 2016

@bors-servo r=Manishearth

@Manishearth Yeah that whole patch is duct tape anyway.

@bors-servo
Copy link
Contributor

📌 Commit 6cc862d has been approved by Manishearth

@highfive highfive assigned Manishearth and unassigned metajack Aug 16, 2016
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 16, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit 6cc862d with merge 2b74e94...

bors-servo pushed a commit that referenced this pull request Aug 16, 2016
Auto merge of #12890 - nox:rng-pressure, r=Manishearth
2b74e94 
Lessen pressure on the thread-local RNG

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

💔 Test failed - linux-rel

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Aug 16, 2016
@highfive
Copy link 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/perspective-origin-001.htm
  └   → /css-transforms-1_dev/html/perspective-origin-001.htm 719003ff0f2c5efe0b678a757a454870ca362e2c
/css-transforms-1_dev/html/reference/ref-filled-green-100px-square.htm d4aa213e3e31e41d0d8e84aec79e94f860f27178
Testing 719003ff0f2c5efe0b678a757a454870ca362e2c == d4aa213e3e31e41d0d8e84aec79e94f860f27178

@nox
Copy link
Contributor Author

nox commented Aug 16, 2016

@bors-servo retry #12891

@bors-servo
Copy link
Contributor

⚡ Previous build results for arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev are reusable. Rebuilding only linux-rel...

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Aug 16, 2016
@highfive
Copy link 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/perspective-origin-004.htm
  └   → /css-transforms-1_dev/html/perspective-origin-004.htm 68d5e8d6d5edcd2bef5ecaaf62cbbbac863dee22
/css-transforms-1_dev/html/reference/ref-filled-green-100px-square.htm d4aa213e3e31e41d0d8e84aec79e94f860f27178
Testing 68d5e8d6d5edcd2bef5ecaaf62cbbbac863dee22 == d4aa213e3e31e41d0d8e84aec79e94f860f27178

@nox
Copy link
Contributor Author

nox commented Aug 16, 2016

@bors-servo retry #12891

@bors-servo
Copy link
Contributor

⚡ Previous build results for arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev are reusable. Rebuilding only linux-rel...

@bors-servo
Copy link
Contributor

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

@bors-servo bors-servo merged commit 6cc862d into servo:master Aug 16, 2016
@nox nox deleted the rng-pressure branch August 16, 2016 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Manishearth Manishearth

Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nox @highfive @Manishearth @bors-servo @metajack