Skip to content

/ servo

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

Jump to bottom

Slowness in EmberPerf #17329

Open
jonathandturner opened this issue Jun 14, 2017 · 7 comments
Open

Slowness in EmberPerf #17329

jonathandturner opened this issue Jun 14, 2017 · 7 comments

Comments

@jonathandturner
Copy link

@jonathandturner jonathandturner commented Jun 14, 2017

I did a comparison of EmberPerf's performance between Servo and the most recent stable Firefox, Safari, and Chrome.

You can see that generally Servo is the slowest of the bunch, though there are a couple places that Servo does decently.

screen shot 2017-06-12 at 14 02 14
@emilio
Copy link
Member

@emilio emilio commented Jun 14, 2017 •
edited

JSC people are ASM wizards.
@emilio
Copy link
Member

@emilio emilio commented Jun 14, 2017

Getting profiles for this could be useful. For example, @glennw filed #17308 this morning, and we already found a bunch of dumb stuff going on (#17315)
@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Jun 14, 2017

@emilio - I'd be happy to help. What's the best way to get a profile?
@emilio
Copy link
Member

@emilio emilio commented Jun 14, 2017

@emilio - I'd be happy to help. What's the best way to get a profile?

perf record works fine on Linux, we have a -p option to set a profile interval, but I think @glennw has reported before it may be lying some times? So perf (or instruments on OSX, maybe?) may be good tools.

I can also take a look if you tell me a good way to get STR... Though I guess it's just loading and running http://emberperf.eviltrout.com/... I can get some profiles.
@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Jun 14, 2017

Yeah, to get it working I just tried running http://emberperf.eviltrout.com/ through all the default tests. Sometimes it takes a couple tries since you might get an error that something isn't defined sometimes.
@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Jun 14, 2017

I'll look into instruments on macOS and see what I can see
@emilio
Copy link
Member

@emilio emilio commented Jun 15, 2017

From a quick look with perf record, it's relatively hard to get some clear numbers on what's slow, apart from the JS engine. The fact that we're using a really old SpiderMonkey doesn't help either... :(

I've seen Stylist::set_device and RuleIterator::next appear in the profile, going to take a look at them. The fact that the style locks still appear around doesn't make me super happy (cc @SimonSapin), but I guess it's fine.
emilio added a commit to emilio/servo that referenced this issue Jun 15, 2017
@emilio
style: Avoid some branches in common RuleIterator::next cases.
Unverified
This user has not uploaded their public key yet.
GPG key ID: 056B727BB9C1027C Learn about signing commits
b41f741 
I saw this function appear in the profiles at servo#17329.

It was under set_device, which isn't a Stylo path, but probably worth there
anyway.

This reduces the reported overhead of RulesIterator::next in perf from ~8% to
0.46%
@emilio emilio mentioned this issue Jun 15, 2017
Merged
bors-servo added a commit that referenced this issue Jun 15, 2017
@bors-servo
Auto merge of #17330 - emilio:rules-iter-perf, r=heycam
Loading status checks…
bb36553 
style: Avoid some branches in common RuleIterator::next cases.

I saw this function appear in the profiles at #17329.

It was under set_device, which isn't a Stylo path, but probably worth there
anyway.

This reduces the reported overhead of RulesIterator::next in perf from ~8% to
0.46%

<!-- 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/17330)
<!-- Reviewable:end -->
@jonathandturner jonathandturner mentioned this issue Jun 19, 2017
Open
0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@jonathandturner @emilio
You can’t perform that action at this time.