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

Use dynamic dispatch in `mozjs::panic::wrap_panic` #26642

Merged
merged 1 commit into from May 26, 2020
Merged

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented May 25, 2020

Pick up servo/rust-mozjs#512

Fixes #26585

This diff is best viewed with "ignore whitespace changes", because of indentation change.

Pick up servo/rust-mozjs#512

Fixes #26585

This diff is best viewed with "ignore whitespace changes", because of indentation change.
@highfive
Copy link

highfive commented May 25, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/bindings/codegen/CodegenRust.py, components/script/script_runtime.rs
  • @KiChjang: components/script/dom/bindings/codegen/CodegenRust.py, components/script/script_runtime.rs
@highfive
Copy link

highfive commented May 25, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
Copy link
Member

Manishearth left a comment

seems good, r=me pending jdm's perf investigation

we could also potentially make it a cfg to use dynamic dispatch here, if there is a small but nontrivial perf impact

@jdm
jdm approved these changes May 26, 2020
Copy link
Member

jdm left a comment

I've verified that performance stays the same on the majority of the demos at https://servo.org/hl-home/. We may be slightly slower at flinging balls as fast as possible in ballshooter, and the particles might also be slightly slower, but both of those were also within the realm of measurement error, so I'm not particularly concerned.

@SimonSapin
Copy link
Member Author

SimonSapin commented May 26, 2020

servo/rust-mozjs#512 has landed

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented May 26, 2020

📌 Commit d103e06 has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented May 26, 2020

Testing commit d103e06 with merge 06c4ca1...

@bors-servo
Copy link
Contributor

bors-servo commented May 26, 2020

☀️ Test successful - status-taskcluster
Approved by: Manishearth
Pushing 06c4ca1 to master...

@bors-servo bors-servo merged commit 06c4ca1 into master May 26, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@bors-servo bors-servo deleted the dynamic-wrap-panic branch May 26, 2020
@bors-servo bors-servo mentioned this pull request May 26, 2020
0 of 5 tasks complete
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.