Bug 987532 (Silk)

[meta] Project Silk on B2G.

RESOLVED WORKSFORME

Status

Firefox OS
Performance
P1
normal
RESOLVED WORKSFORME
3 years ago
2 years ago

People

(Reporter: vilin, Assigned: jerry)

Tracking

(Depends on: 2 bugs, {perf})

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(tracking-b2g:+)

Details

(Whiteboard: [c=uniformity p= s= u=][ucid:graphic17,ft:graphic], URL)

User Story

As a User, when I move a finger across the screen at a constant rate (? pixels/sec), frame displacement should be constant, so I have a good visual user experience.
The Std deviation of the frame displacement should  <= 5 pixels.

Info on how to measure at https://wiki.mozilla.org/FirefoxOS/Performance/Graphics/UniformityMeasure

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
The idea of Project Butter is to make display smoother. It includes vsync monitor/observer manager/observers, input event resampling, vsync-triggered RefreshDriver timer, vsync-triggered compositor and triple-buffering.

This meta bug is used to trace these modifications belongs to difference modules.

Bug 980241 - Vsync-triggered RefreshDriver
Bug 987523 - Vsync-triggered CompositorParent
Bug 970751 - Deliver input events more consistently per composite to make scrolling smoother
Bug 987527 - Register Vsync monitor to HWComposer and notify to gecko/hal 
Bug 987529 - Implement an observer manager for vsync in gecko/hal

Comment 1

3 years ago
Very nice. Please make sure to move all the gfx work into the Graphics component and the input events stuff should be in Widget:Gonk.
Depends on: 846535

Updated

3 years ago
Keywords: perf
Whiteboard: [c= p= s= u=]
(Reporter)

Updated

3 years ago
Depends on: 988160

Updated

3 years ago
Whiteboard: [c= p= s= u=] → [c=uniformity p= s= u=]

Updated

3 years ago
Priority: -- → P2
Alias: butter

Updated

3 years ago
Depends on: 990832

Updated

3 years ago
Depends on: 991420
(Reporter)

Updated

3 years ago
Depends on: 991483
(Reporter)

Comment 2

3 years ago
design document
https://etherpad.mozilla.org/vsync-butter
(Reporter)

Updated

3 years ago
No longer depends on: 970751
(Reporter)

Updated

3 years ago
No longer depends on: 987527
(Reporter)

Updated

3 years ago
No longer depends on: 987529
(Reporter)

Updated

3 years ago
Alias: butter → Vsync

Updated

3 years ago
Alias: Vsync → Silk

Updated

3 years ago
See Also: → bug 965272

Updated

3 years ago
Assignee: vlin → hshih

Updated

3 years ago
Blocks: 1016796
feature-b2g: --- → 2.1
Whiteboard: [c=uniformity p= s= u=] → [c=uniformity p= s= u=][ucid:graphic17,ft:graphic]
Summary: [meta] Project Butter on B2G. → [meta] Project Silk on B2G.

Updated

3 years ago
Depends on: 930939

Updated

3 years ago
Depends on: 982972
Add bug 987529 into dependency list.

All the following work of SiLK is based on implementation of bug 987529
Depends on: 987529
Add bug 930939 in dependency list.

In attachment 8437591 [details], I try to describe why Input handling OOMT is important for SiLK.

Comment 5

3 years ago
This bug has B2G at its title, but desktop would have a lot to gain from such system as well (windows specifically, because OS X already blocks on glSwap even without perfect timing, and I'm not sure about Linux).

How much does the work here apply for desktop Firefox? If reasonably but not fully, is there a desktop equivalent of this bug/plan?
(In reply to Avi Halachmi (:avih) from comment #5)
> This bug has B2G at its title, but desktop would have a lot to gain from
> such system as well (windows specifically, because OS X already blocks on
> glSwap even without perfect timing, and I'm not sure about Linux).
> 
> How much does the work here apply for desktop Firefox? If reasonably but not
> fully, is there a desktop equivalent of this bug/plan?

Hi Avi,
Here is vsync dispatch flow attachment 8429109 [details].
There are two platform dependent objects
1. (HW vsync)The HWComposer rectangle. at the bottom of this diagram, is B2G only. On B2G, HWComposer receive vsync hw event and dispatch to the client - VsyncDispatcher.
To porting this mechanism on different platforms, we need to construct another back-end to receive HW vsync event and poke VsyncDispatcher at right time. 
2. (Vsync observer) InputDispatcher - each nsAppShell has its own way of input event handling. 

Current plan is to make it work and fine tune on B2G first. After that, then we will move forward to the other platforms.

Comment 7

3 years ago
Thanks for the info. Any rough plan/hope for the timelines of this? E.g. are we hoping to start looking at desktops in weeks? months? years?
https://wiki.mozilla.org/B2G/Roadmap
1. B2G 2.1 land project silk with preference off.
2. B2G 2.2 turn on by default.

Ideally, most engineer work should be done in milestone #1(due: Oct 13, 2014). Depend on the status of project silk in the end of 2.1, we can make a decision whether kick off platform support at that moment. Is this make sense to you?

Comment 9

3 years ago
I was just trying to get a rough idea of the roadmap, which you provided (and I like that it seems at the block diagram that many components are shared between platforms - at least at this level).

If it makes sense or not has a lot to do with resources and priorities, and I'm probably unaware of enough of those to answer that. If you were asking for my opinion about focusing on one platform first, then it sounds like a good plan, and it's probably also worth bumping it against the desktop gfx teams to make sure it will fit nicely, which I'm sure you've done as well.

Comment 10

3 years ago
Hi Peter/CJ: Can we define some benchmarks/metrics for Project Silk for 2.1?
ni myself to provide the measurement of project silk.
Flags: needinfo?(pchang)

Updated

3 years ago
User Story: (updated)
(In reply to Sandip Kamat from comment #10)
> Hi Peter/CJ: Can we define some benchmarks/metrics for Project Silk for 2.1?

Since Project Silk is off by default for 2.1, I think we should meet the following conditions.
a. Green on try server
b. No performance impact, like app launch time and scrolling
c. Green on try server when SW vsync(bug 1043822) turns on

In 2.2, Project Silk is on by default, we not only need to meet above conditions but also the following items.

d. Frame uniformity

Mason and Jerry, any items need to be considered for Project Silk?
Flags: needinfo?(pchang)
Flags: needinfo?(mchang)
Flags: needinfo?(hshih)

Updated

3 years ago
Depends on: 1043822

Updated

3 years ago
Depends on: 987527

Updated

3 years ago
Depends on: 1035076

Updated

3 years ago
Hi Peter,

When you say (c), do we have any requirements for hardware vsync or can it be landed by 2.1?

Other than that, it looks fine by me. We have some difficulty in measuring (d), but as long as we have improved uniformity, I'm ok with that requirement. I don't have a good answer yet on a number on exactly how to measure uniformity.

Thanks!
Flags: needinfo?(mchang)

Updated

3 years ago
QA Whiteboard: [2.1-feature-qa+]

Updated

3 years ago
Depends on: 1048667
(In reply to Mason Chang [:mchang] from comment #13)
> Hi Peter,
> 
> When you say (c), do we have any requirements for hardware vsync or can it
> be landed by 2.1?
> 
> Other than that, it looks fine by me. We have some difficulty in measuring
> (d), but as long as we have improved uniformity, I'm ok with that
> requirement. I don't have a good answer yet on a number on exactly how to
> measure uniformity.
> 
> Thanks!

For the item c, it tries to verify the Project Silk with SW vsync on try server, like b2g emulator. So far we don't have real device with KK on try server.
Jerry, let's set the design document/wiki page to this bug as attachment, and then ask for a review from :jrmuizel, :bas and :roc for it.  That way we can track the r+ for the design in a single place.

Updated

3 years ago
Flags: in-moztrap?(npark)

Comment 16

3 years ago
(In reply to peter chang[:pchang][:peter] from comment #12)
> (In reply to Sandip Kamat from comment #10)
> > Hi Peter/CJ: Can we define some benchmarks/metrics for Project Silk for 2.1?
> 
> Since Project Silk is off by default for 2.1, I think we should meet the
> following conditions.
> a. Green on try server
> b. No performance impact, like app launch time and scrolling
> c. Green on try server when SW vsync(bug 1043822) turns on
> 
> In 2.2, Project Silk is on by default, we not only need to meet above
> conditions but also the following items.
> 
> d. Frame uniformity
> 
> Mason and Jerry, any items need to be considered for Project Silk?


Given that and b need to happen anyways, and c adds value, Does this effectively mean "phase 1" scope in 2.1 is mostly around bug 1043822?
I like the 2.1 definition in comment 12, just would like to be a bit more explicit, and check if one more thing is possible.  So, for 2.1:

a) with sync pref'd off, there is no performance regression on any platform/configuration (including those with hardware composer).  There are also no try regressions (or it couldn't land.)
b) with sync pref'd on, and SW Vsync option, try is green, and there are no performance regressions on platforms without hardware composer.

Now, we would need to find room for the hardware Vsync (is it landed, but with some performance regressions, or not landed at all?), as you mention actual frame uniformity (which we will define the goals hopefully as something that can be assigned a number or two), and whatever else is needed.

Updated

3 years ago
QA Contact: npark

Updated

3 years ago
QA Whiteboard: [2.1-feature-qa+]
Whiteboard: [c=uniformity p= s= u=][ucid:graphic17,ft:graphic] → [c=uniformity p= s= u=][ucid:graphic17,ft:graphic][2.1-feature-qa+]
(Assignee)

Comment 18

3 years ago
Created attachment 8471589 [details]
silk design wiki

This the silk project design wiki.
Please review this design wiki.
Then we can go to review the vsync dispatcher framework in bug 1048667.
Attachment #8471589 - Flags: review?(roc)
Attachment #8471589 - Flags: review?(jmuizelaar)
Attachment #8471589 - Flags: review?(bas)

Comment 19

3 years ago
(In reply to Milan Sreckovic [:milan] (PTO 8/11 - 8/15) from comment #17)
> I like the 2.1 definition in comment 12, just would like to be a bit more
> explicit, and check if one more thing is possible.  So, for 2.1:
> 
> a) with sync pref'd off, there is no performance regression on any
> platform/configuration (including those with hardware composer).  There are
> also no try regressions (or it couldn't land.)
> b) with sync pref'd on, and SW Vsync option, try is green, and there are no
> performance regressions on platforms without hardware composer.
> 
> Now, we would need to find room for the hardware Vsync (is it landed, but
> with some performance regressions, or not landed at all?), as you mention
> actual frame uniformity (which we will define the goals hopefully as
> something that can be assigned a number or two), and whatever else is needed.

This makes sense.  Peter, Can you help list the phase 1 items and benchmarks for this? Let's open a separate bug for this and phase 2. possibly we need to group the bugs listed on wiki for 2 meta bugs.
Flags: needinfo?(pchang)
(In reply to Sandip Kamat from comment #19)
> (In reply to Milan Sreckovic [:milan] (PTO 8/11 - 8/15) from comment #17)
> > I like the 2.1 definition in comment 12, just would like to be a bit more
> > explicit, and check if one more thing is possible.  So, for 2.1:
> > 
> > a) with sync pref'd off, there is no performance regression on any
> > platform/configuration (including those with hardware composer).  There are
> > also no try regressions (or it couldn't land.)
> > b) with sync pref'd on, and SW Vsync option, try is green, and there are no
> > performance regressions on platforms without hardware composer.
> > 
> > Now, we would need to find room for the hardware Vsync (is it landed, but
> > with some performance regressions, or not landed at all?), as you mention
> > actual frame uniformity (which we will define the goals hopefully as
> > something that can be assigned a number or two), and whatever else is needed.
> 
> This makes sense.  Peter, Can you help list the phase 1 items and benchmarks
> for this? Let's open a separate bug for this and phase 2. possibly we need
> to group the bugs listed on wiki for 2 meta bugs.

Sandip, please check the Project Silk wiki page and you will see bugs with target milestone. I think it is enough for us to track Project Silk in 2.1.

About the benchmarks, Project Silk tries to align Vsync for every rendering operation to provide better user experience, like smoother scrolling, but not tries to boost the scrolling performance. Project Silk is on for 2.2, therefore, I would not expect to see the benchmark number for 2.1 although we already have better frame uniformity now and we will continue improving it after 2.1.

Besides frame uniformity, we also try to create a app to draw a small ball which follows with your finger. We can measure the distance between fingers(touch) and the position of the small ball for 2.2. The detail of benchmark will be listed at the benchmark section of Project Silk wiki page.
Flags: needinfo?(pchang)
Flags: needinfo?(hshih)
 > Besides frame uniformity, we also try to create a app to draw a small ball
> which follows with your finger. We can measure the distance between
> fingers(touch) and the position of the small ball for 2.2. The detail of
> benchmark will be listed at the benchmark section of Project Silk wiki page.

Hi Peter, sorry I'm confused,  how would this work? From touch interpolation, we'll always be behind the actual touch event, so we should be smoother while following the touch but increase in distance. Could we also have an animation benchmark? If vsync and composites are within 16 ms, we should see smoother OMTA as well right? Thanks!
Testing Perspective:

minused in-moztrap flag because there is no manual test case to verify animation is 'smoother.'  This feature will be verified with following methods:
For 2.1:
- Manual graphics smoke test suite, checking for possible regression when the feature is preffed off
For 2.2:
- Manual graphics smoke test suite, checking for possible regression when the feature is preffed on/off
- Frame uniformity tool (bug 990832)
- Check for consistent composite interval of approximately 16.6 ms ( https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler)
Flags: in-moztrap?(npark) → in-moztrap-
Attachment #8471589 - Flags: review?(matt.woodrow)
The current design has VsyncThread making a sync call to the MainThread to run the InputDispatcher task. It's not clear to me why InputDispatcher has to run on the main thread, and it seems to me things would be a lot better if it didn't have to. So please document somewhere why InputDispatcher has to run on the MainThread.

Assuming that has to stay the same, what's the point of having the VsyncThread? If I understand the diagrams correctly, HWcomposer sends an async message to the VsyncThread to start the vsync process. If it can send an async message to the MainThread instead, we could go directly into the InputDispatcher and avoid a context switch.

Also, it looks like we don't need to return to the VsyncThread after the InputDispatcher has finished. The MainThread could itself send async messages to the CompositorThread and the NotifyVsyncEvent listeners when InputDispatcher finishes. This would eliminate another context switch.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23)
> The current design has VsyncThread making a sync call to the MainThread to
> run the InputDispatcher task. It's not clear to me why InputDispatcher has
> to run on the main thread, and it seems to me things would be a lot better
> if it didn't have to. So please document somewhere why InputDispatcher has
> to run on the MainThread.

iirc, the APZ event processing code all currently runs on the main thread. We need that to run to completion, and forward any required input events to the child processes before we send the vsync notification that will trigger the refresh driver.

I assume the VsyncThread exists for the future where this processing will happen on that thread, without MainThread involvement. Then we'd only have async notifications coming from the VsyncThread.

Assuming that's all correct, it would be nice if it were more clear that the synchronous call to the MainThread is a temporary thing, explaining why it exists and how it will be fixed.
(In reply to Matt Woodrow (:mattwoodrow) from comment #24)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23)
> > The current design has VsyncThread making a sync call to the MainThread to
> > run the InputDispatcher task. It's not clear to me why InputDispatcher has
> > to run on the main thread, and it seems to me things would be a lot better
> > if it didn't have to. So please document somewhere why InputDispatcher has
> > to run on the MainThread.
> 
> iirc, the APZ event processing code all currently runs on the main thread.
> We need that to run to completion, and forward any required input events to
> the child processes before we send the vsync notification that will trigger
> the refresh driver.
> 
> I assume the VsyncThread exists for the future where this processing will
> happen on that thread, without MainThread involvement. Then we'd only have
> async notifications coming from the VsyncThread.
> 
> Assuming that's all correct, it would be nice if it were more clear that the
> synchronous call to the MainThread is a temporary thing, explaining why it
> exists and how it will be fixed.

Correct, we currently have a plan to get APZ off the main thread, this is in bug 930939. We currently want to process all touch events on the VsyncThread as we did not want to make a new input thread, which android already has.

The ideal architecture would be that we first sync notify APZ, then notify the Compositor. Thus, when we composite, we will have the most up to date touch coordinates rather than have a racy condition where we might process the previous touch event versus the latest touch events when we composite. This should reduce touch latency for the user. Please let me know if you have any questions. Thanks!
(Assignee)

Comment 26

3 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23)
> The current design has VsyncThread making a sync call to the MainThread to
> run the InputDispatcher task. It's not clear to me why InputDispatcher has
> to run on the main thread, and it seems to me things would be a lot better
> if it didn't have to. So please document somewhere why InputDispatcher has
> to run on the MainThread.
bug 930939 will do the dispatch off main thread.
> 
> Assuming that has to stay the same, what's the point of having the
> VsyncThread? If I understand the diagrams correctly, HWcomposer sends an
> async message to the VsyncThread to start the vsync process. If it can send
> an async message to the MainThread instead, we could go directly into the
> InputDispatcher and avoid a context switch.

The latency between using HWC or VsyncThread to dispatch the input is quite small.
If we dispatch in VsyncDispatcher, the code is easy to read.

> 
> Also, it looks like we don't need to return to the VsyncThread after the
> InputDispatcher has finished. The MainThread could itself send async
> messages to the CompositorThread and the NotifyVsyncEvent listeners when
> InputDispatcher finishes. This would eliminate another context switch.

If chrome main thread is very busy, the main thread can't handle each vsync event.
The compositor or content can't receive vsync event anymore.
In current implementation, I use a monitor with 2ms timeout to wait the main thread input processing.
If the monitor timeout, we continue to notify compositor and content process.
Flags: needinfo?(roc)
(Assignee)

Updated

3 years ago
No longer depends on: 988160
(Assignee)

Updated

3 years ago
No longer depends on: 846535
If bug 930939 will eliminate the main thread from the event processing path and process those events on the VsyncThread instead, that sounds great. But as Matt said, please adjust your documentation to explain what the final state is going to be, with a diagram. Thanks!
Flags: needinfo?(roc)
(Assignee)

Comment 28

3 years ago
Thanks, roc and matt.

Please check:
https://wiki.mozilla.org/Project_Silk#Order_to_dispatch_Vsync_events

We have the diagram for current design and the ideal model with bug 930939.
Flags: needinfo?(roc)
Flags: needinfo?(matt.woodrow)

Updated

3 years ago
feature-b2g: 2.1 → 2.2
Looks good thanks!
Flags: needinfo?(roc)

Updated

3 years ago
Whiteboard: [c=uniformity p= s= u=][ucid:graphic17,ft:graphic][2.1-feature-qa+] → [c=uniformity p= s= u=][ucid:graphic17,ft:graphic]

Comment 30

3 years ago
Use feature-b2g:2.2? rather than just 2.2.
feature-b2g: 2.2 → 2.2?
Depends on: 1062117

Updated

3 years ago
Blocks: 1071275

Updated

3 years ago
Depends on: 1072627

Updated

3 years ago
Depends on: 1073545
Depends on: 1073704

Updated

3 years ago
Depends on: 1042017
Just a note, we require Kit-Kat for silk. This has to be a technical requirement. On JellyBean on both Nexus 4 and Flame, vsync intervals coming from the drivers were inconsistent. e.g., sometimes a vsync would come in at 32 ms or 4 ms instead of standard 16.6ms intervals, which would, by design, create janky and worse behavior than we currently have. So far from our testing, Kit-Kat, v184 Flame has fixed this.

Updated

3 years ago
Depends on: 1077651
(Assignee)

Updated

3 years ago
Depends on: 1078136
(Assignee)

Updated

3 years ago
Depends on: 1078152
(Assignee)

Updated

3 years ago
Depends on: 1078157
(Assignee)

Updated

3 years ago
Depends on: 1078160
(Assignee)

Updated

3 years ago
Depends on: 1078168
(Assignee)

Updated

3 years ago
Depends on: 1078171

Comment 32

3 years ago
Referred here by avihpit in #894128 regarding a vsync issue.  Please validate and verify any Silk vsync implementation against this testing tool:

   http://www.duckware.com/test/chrome/jerky3.html

Updated

3 years ago
Depends on: 1083530
(Assignee)

Updated

3 years ago
Flags: needinfo?(matt.woodrow)
(Assignee)

Updated

3 years ago
Attachment #8471589 - Flags: review?(roc)
Attachment #8471589 - Flags: review?(matt.woodrow)
Attachment #8471589 - Flags: review?(jmuizelaar)
Attachment #8471589 - Flags: review?(bas)

Updated

3 years ago
Depends on: 1092245
(Assignee)

Updated

3 years ago
Depends on: 1092978

Updated

3 years ago
Depends on: 1095242
Depends on: 1098701

Updated

3 years ago
Depends on: 1101974

Updated

3 years ago
Depends on: 1102631
Depends on: 1094760
Depends on: 1102200
Depends on: 1062331
No longer depends on: 1078157
No longer depends on: 1102631
No longer depends on: 1077651

Updated

3 years ago
QA Whiteboard: [2.2-feature-qa+]

Updated

3 years ago
No longer blocks: 1016796

Comment 33

3 years ago
Bobby, can you follow up this topic? Thanks.
feature-b2g: 2.2? → ---
Flags: needinfo?(bchien)
manage in feature-b2g for future release
tracking-b2g: --- → +
Flags: needinfo?(bchien)
Priority: P2 → P1
Depends on: 1102631

Updated

3 years ago
Depends on: 1117870

Updated

3 years ago
Depends on: 1118530
(Assignee)

Updated

2 years ago
Depends on: 1123734
(Assignee)

Updated

2 years ago
Depends on: 1123762

Updated

2 years ago
Duplicate of this bug: 689418

Updated

2 years ago
Depends on: 1130678
(Assignee)

Updated

2 years ago
Depends on: 1139253
Silk on b2g is on by default in master and now uplifted to 2.2.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.