Repository navigation
perf: add initial threadboost mechanism (for ohos). - #46843
Conversation
The actual design will need to be iterated on, especially since the implementation might diverge strongly based on the target platfrom. On non heterogeneous platforms we probably don't need to do anything, since all cores are equal. On a harmonyos device the spread-out nature of servo, with threads calling out to other threads, doesn't interact well with the default scheduler, since the threads on the critical path might not be busy enough (consecutively) to be boosted to a medium or large core. Since the little cores on this device are ~1/3 as capable as the large ones (according to the sysfs capability metric) this can have a large impact on performance. With all threads boosted to medium cores, we get a moderate 8% reduction in LCP on wikipedia. For the reproducer in servo#45199 it is very noticable and boosts the framerate to 60 fps, where it is capped. With active touch input which raises the display frequency to 120hz, we reach ~74fps. Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
|
🔨 Triggering try run (#30396696567) for OpenHarmony |
|
Bencher Report| Project | Servo |
| Branch | 46843/PR |
| Testbed | HUAWEI Mate 60 Pro |
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
|
✨ Try run (#30396696567) succeeded. |
|
|
||
| // After init_app so the thread affinity mask doesn't affect all children spawned in init_app. | ||
| servo_base::threadboost::mark_thread_as_critical(); | ||
|
|
There was a problem hiding this comment.
We should not be exposing any new public API from crates that are not servo. Couldn't this happen internally in Servo when the Servo instance is created?
There was a problem hiding this comment.
Hmm, I'm not sure. I'd be wary of just modifying the embedder thread from in servo without the embedder opting in. I think we'd need to have the full discussion about how the final design abstraction should look like first (which probably includes experimenting first on multiple different heterogeneous targets (android, mobile Linux, mobile windows?) to figure out what works and what is required.
Would you be fine with just re-exporting this from servo for now?
Edit: oh and the M macs also have effeciency cores - would probably also be interesting to check on what cores servo is scheduled there.
There was a problem hiding this comment.
Maybe this could be a preference like set_thread_affinities. I think a preference is nicer for embedder rather than knowing that they have to call a function to make Servo go fast. Then we can implement this for other platforms.
There was a problem hiding this comment.
Maybe this could be a preference like set_thread_affinities
I would prefer to defer a public API like this until we have done experimentation with what works / doesn't work on other platforms, since it will likely change the shape of this API (right now it's basically "this works on ohos"), but we should try and see what works on android, arm windows, arm mac and mobile linux.
There was a problem hiding this comment.
I'm not sure I understand. The new mark_thread_as_critical API is also public.
There was a problem hiding this comment.
I'm considering marking the re-export as #[doc(hidden)] for now.
I think a preference is nicer for embedder rather than knowing that they have to call a function to make Servo go fast
I think a serious embedder should be aware of this, since they should also be able to customize the policy (of what exactly happens for critical threads). Also worth noting that by pinning most servo threads to larger cores, the embedder thread also is more likely to scheduled there, since the scheduler will prefer free hot cores I believe.
It may be that an embedder does not want the main thread to have custom thread affinities, but does want servos thread to prefer larger cores.
My rough idea would be to add a section to "Embedding Servo" in the servo book explaining the mechanism (and trade-offs), after we come up with a generalization.
There was a problem hiding this comment.
Maybe this could be a preference like set_thread_affinities
I would prefer to defer a public API like this until we have done experimentation with what works / doesn't work on other platforms, since it will likely change the shape of this API (right now it's basically "this works on ohos"), but we should try and see what works on android, arm windows, arm mac and mobile linux.
We could also expose this as a preference now and mark the preference with #[doc(hidden)].
There was a problem hiding this comment.
Okay - I added a pref to toggle the feature and moved the embedder thread boost to servo initialization.
I'm planning to iterate on this and probably the embedder thread boost will disappear again, since it seems like the important threads are all child threads (from webrender), which could / should be boosted individually.
|
I tested a quick implementation for Linux based on the gdt_cpus crate and I got similar improvements on speedometer. My Macbook Air M1 running Fedora (4 performance cores and 4 efficiency cores) moves from a score of 4.20 to 7.71 So I would say the sooner we get that change in, the better :) |
|
Can confirm that for most cases speedometer on harmonyos shows improvement. So I think this should be merged. |
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
mrobinson
left a comment
There was a problem hiding this comment.
Looks good with a few nits:
Co-authored-by: Martin Robinson <martin@abandonedwig.info> Signed-off-by: Jonathan Schwender <55576758+jschwe@users.noreply.github.com>
servo#47027) Follow-up to servo#46843. Instead of boosting the embedder thread, which happened to also boost the webrender (children) threads (threads spawned after we changed the thread affinity for the embedder thread), boost the webrender threads explicitly and remove the embedder thread boost. This seems to have the same net performance, hence it's better to avoid changing the properties of the embedder thread. Additionally modified the boosting API, so that we can specify a priority level, and opt-in to the thread affinity boosting. This gives us more flexibility, since some threads might just want one or the other. In the future this might also be useful if we want to de-prioritize certain threads. Testing: We don't have any automated tests measuring performance or thread placement. --------- Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
The abstraction design will need to be iterated on, especially since the implementation might diverge strongly based on the target platfrom. On non heterogeneous platforms, we probably don't need to do anything, since all cores are equal. Background: On a harmonyos device the spread-out nature of servo, with threads calling out to other threads, doesn't interact well with the default scheduler, since the threads on the critical path might not be busy enough (consecutively) to be boosted to a medium or large core. Since the little cores on this device are ~1/3-1/4 as capable as the medium/large ones (according to the sysfs capability metric) this can have a large impact on performance. Improvements on ohos: - a moderate 8% reduction in LCP on wikipedia. - For the reproducer in servo#45199 it is very noticable and boosts the framerate to 60 fps, where it is capped. With active touch input which raises the display frequency to 120hz, we reach ~74fps. In speedometer on my test-device the test score improves by ~50%. This PR should be seen as a first step towards servo#46813. The abstraction itself as is, is not great, but partially also can't be avoided since mechanisms will diverge depending on the platform. Part of the reason this PR works as well as it does, despite only explicitly prioritising 3 threads, is that all child threads spawned after the thread affinity mask takes effect inherit the affinity mask (which is an implementation detail of the ohos platform, since other platforms likely would not want to use affinity masks). The thread priority set via `OH_QoS_SetThreadQoS` only affects the current thread and not any children. Note: I also checked, and notably chromium on ohos doesn't use thread affinity masks. However, it does more work on the main thread, which seems to interact better with the scheduler heuristics since it is boosted to medium cores under load (or perhaps the scheduler is aware of important chromium threads, since it is an important system app). Testing: We currently don't measure production performance before merging to main. --------- Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com> Signed-off-by: Jonathan Schwender <55576758+jschwe@users.noreply.github.com> Co-authored-by: Martin Robinson <martin@abandonedwig.info>
servo#47027) Follow-up to servo#46843. Instead of boosting the embedder thread, which happened to also boost the webrender (children) threads (threads spawned after we changed the thread affinity for the embedder thread), boost the webrender threads explicitly and remove the embedder thread boost. This seems to have the same net performance, hence it's better to avoid changing the properties of the embedder thread. Additionally modified the boosting API, so that we can specify a priority level, and opt-in to the thread affinity boosting. This gives us more flexibility, since some threads might just want one or the other. In the future this might also be useful if we want to de-prioritize certain threads. Testing: We don't have any automated tests measuring performance or thread placement. --------- Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
The abstraction design will need to be iterated on, especially since the implementation might diverge strongly based on the target platfrom. On non heterogeneous platforms, we probably don't need to do anything, since all cores are equal.
Background: On a harmonyos device the spread-out nature of servo, with threads calling out to other threads, doesn't interact well with the default scheduler, since the threads on the critical path might not be busy enough (consecutively) to be boosted to a medium or large core. Since the little cores on this device are ~1/3-1/4 as capable as the medium/large ones (according to the sysfs capability metric) this can have a large impact on performance.
Improvements on ohos:
In speedometer on my test-device the test score improves by ~50%.
This PR should be seen as a first step towards #46813. The abstraction itself as is, is not great, but partially also can't be avoided since mechanisms will diverge depending on the platform. Part of the reason this PR works as well as it does, despite only explicitly prioritising 3 threads, is that all child threads spawned after the thread affinity mask takes effect inherit the affinity mask (which is an implementation detail of the ohos platform, since other platforms likely would not want to use affinity masks). The thread priority set via
OH_QoS_SetThreadQoSonly affects the current thread and not any children.Note: I also checked, and notably chromium on ohos doesn't use thread affinity masks. However, it does more work on the main thread, which seems to interact better with the scheduler heuristics since it is boosted to medium cores under load (or perhaps the scheduler is aware of important chromium threads, since it is an important system app).
Testing: We currently don't measure production performance before merging to main.