1117050 - [FFOS7715 v2.1][performance]It is slow to unlock FFOS2.1 .

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

Description

[jingmei.zhang]

drag the icon to unlock the screen,note down the time from our finger leave the screen to the main screen is show.

it is slower than FFOS 1.4.
time used in 1.4:0.956
time used in 2.1:1.265

Comment 1

[zhaoyang]

Hi Greg:

I know you epxpected in lockscreen ! Could you help me ?

I want to reduce the unlock time and I try to remove lockscreen‘s the CSS attribute directly , and send the lockscreen-appclosing and lockscreen-appclosed .etc events, but there are two problem  after unlock:

1. when the rotating screen, lockscreen response to ‘system-resize‘, not APP

2. when lock screen again, found the lockscreen does not show, unless I set the active .etc CSS attribute directly

I think the lockscreen in a state of the life cycle of APP after unlock, but I don't know the state.

Could you give me some guidance?

Please help me!

Thanks a lot!

Comment 2

[Greg Weng [:snowmantw][:gweng][:λ]]

Could you clarify you questions?

1. system-resize is necessary since if we don't do that, device would show a half-size LockScreen in my recollection. I don't know why you think this is a bug.

2. I don't know why it doesn't show since I don't know why and how you remove the rules. If you don't give me the details I can never know what's broken.

3. I don't know what the "life cycle of APP after unlock" means.

Comment 3

[zhaoyang]

Hi Greg:

Sorry！

I mean to use my method to unlock，there will two problems. It's not a root-caused bug.

I do that，because I want to reduce the unlocking time

I hope you can give me some guidance.

‘lockscreen’ will go from 'opened' to 'closed' as a real APP，when unlocking the phone.

Now,I try to remove lockscreen‘s the CSS attribute directly instead of "this.states.instance.close(instant ? 'immediate': undefined);" , when unlocking the phone.

Use my method to unlock，there will two problem：

1. when the rotating screen, lockscreen response to ‘system-resize‘, not APP.(phone is unlocked)

2. when lock screen again, found the lockscreen does not show, unless I set the active .etc CSS attribute of 'lockscreen' directly

Could you give me some guidance?
Thanks a lot!

Comment 4

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Just have an offline discussion with David. Let me rephrase his ideas:

Purpose: To enhance the FireFox OS v2.1 unlock time in order to meet SPRD internal performance criteria

Proposal: In the current FireFox OS design, although the lockscreen is not a applicatyion per se, we still treat it like a application such that it experiences the application life cycle. However, one drawback of this is it will prolong the time needed to unlock the screen since the application life cycle involves some time consuming tasks such as display animation.

And since the lockscreen is an application that rarely interact with other application, I try to speed up the unlock procedure but not calling the |close| method in the app_window.js, but directly modify the classlist/attribute of the lockscreen. My modification in lockscreen_window_manager.js is as following:

In closeApp()：
      //this.states.instance.close(instant ? 'immediate': undefined);
      window.dispatchEvent(
              new CustomEvent('lockscreen-appclosing'));
      this.states.instance.element.setAttribute('transition-state', 'closed');
      this.states.instance.setVisible(false);
      this.states.instance.element.classList.remove('active');
      window.dispatchEvent(
              new CustomEvent('lockscreen-appclosed'));
      
In openApp()：
      this.states.instance.element.setAttribute('transition-state', 'opened');
      this.states.instance.element.classList.add('active');
      this.states.instance.element.setAttribute('aria-hidden', 'false');

I understand that this is hacky and not orthodox, it is just that I am new to FFOS and i am assign to the task to enhance the unlock time, so I will be much appreciated if you can kindly check my modification and feedbacks

Thanks

So Greg, could you kind help to comment?

Thanks

Comment 5

[Greg Weng [:snowmantw][:gweng][:λ]]

I'm afraid your comment "lockscreen is an application that rarely interact with other application" may be incorrect. LockScreen *do* lots of interactions with System app, which mediates all apps with lots of effects like resizing, animation, hide things and show things while unlocking and locking. It's even has a bit more interactions with System app than other ordinary apps.

And I must ask what makes you feel the root cause is we follow the app lifecycle to implement LockScreenWindow. Since there is no profiling result shows the root cause is AppWindow-related behaviors (welcome to submit it and we can discuss the issue according to that), but the whole information here and what your proposal based, at least according to what this Bug page shows, is just someone commented some "benchmark" I don't even know how he/she estimated that, and you just quickly (maybe under some notorious pressures I could understand) ask me what's my feeling about the proposal. This is not the usual way to hack architecture, not just several lines of code.

Moreover, I think there should be lots of things can be enhanced, not to mention we don't need to assume Gecko is 100% perfectly so there are so many issues are actually from platform so all Gaia works are dirty workarounds that may also downgrade the performance, and of course the final solution should be Gecko patches, not more Gaia workarounds that you and me would never want to maintain that after 3 months later. So right now I'm sorry that I could only never agree anything take so much risks at a branch after a long while of stabilization just with such so few information. If you have anything firm about the reason we must take such action, in other words the risk is definitely worth, you can post it here and we could discuss it.

(NI Tim, Alive since this would be a System-wide change)

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I would like to see more information as well, e.g. timeline showing that we are making __unnecessary__ pauses.

If it's proven, this can be a generic bug that should be improved on master as well.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Same as Tim. I don't see the value to adopt the hack like comment 4 - unless you are doing your local changes only in your branch. Also I don't see how much time comment 4 is saving. Don't know how to feedback.

Comment 8

[zhaoyang]

Hi Greg、Tim and Alive:

   2.1 is slower than FFOS 1.4.
   time used in Andriod:0.393
   time used in 1.4:0.956
   time used in 2.1:1.265
   Our QA test by high speed camera capture time from ‘finger leave’ to ‘homescreen show’.

   I calculate time consuming from ‘closeApp’ started to ‘closeApp’ completed and list three times contrast:
   before modify:0.10s  0.11s  0.17s
   after  modify:0.03s  0.04s  0.04s

   I can't find '__unnecessary__ pauses' as Alive said, and I think lockscreen *do* lots of interactions with System app through dispatch 'lockscreen-appclosing'、‘lockscreen-appclosed’ and ‘lockscreen-appclose’ event. In My modification I also dispatch these events just as v_1.4.

   I re modified and solved problems in comment 4. My new modification in lockscreen_window_manager.js is as following:

In closeApp()：
-      this.states.instance.close(instant ? 'immediate': undefined);
+      window.dispatchEvent(
+              new CustomEvent('lockscreen-appclosing'));
+      this.states.instance.element.setAttribute('transition-state', 'closed');
+      this.states.instance.transitionController._transitionState = 'closed';
+      this.states.instance.setVisible(false);
+      this.states.instance.element.classList.remove('active');
+      window.dispatchEvent(
+              new CustomEvent('lockscreen-appclosed'));      
+      window.dispatchEvent(
+              new CustomEvent('lockscreen-appclose'));

    
    Thanks for your comment and I learn much from it!
    Maybe my modification is not good and can't save time much, But please give more attention to time consumed when unlock the screen.

    Thanks a lot!

Comment 9

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Greg, Tim, Alive, thanks for the comments and suggestions.

Hi David -

As pointed out, we think your modification might be based on some wrong assumptions of the Lockscreen mechanism. We still think the best way to address this kind of performance issue is to do the profiling first, identify the bottleneck, and then try to enhance the performance from a system/architecture perspective. 

And we understand that you are under tremendous pressures to enhance the performance and meet the project schedule. You probably won't have enough time to come out a perfect solution. Hence it is entirely up to you to decide whether to put this fix in your project branch and benefit from the performance enhancement it will bring, and take the risks of any potential side effects it might introduce as well.

Thanks for your help

Comment 10

[Greg Weng [:snowmantw][:gweng][:λ]]

David: Changes of course bring some improvements, or the change would be meaningless. However, I think what we should take is to profile the unlocking moment to give us more information and focus on the app-lifecycle to find out what makes it's slow, rather than just give a solution to avoid it (so it would be a hack, not a real solution). It's important to have this knowledge, since as I said, we must have the knowledge to prevent we keep ignorant in the future, which would make us in troubles while we need to maintain such code. And the past experience shows the time to repay such technical debt always come very soon, so even we at last decide to land this only on 2.1S is OK, to keep ignorant with such change is still dangerous, especially when you finally run into some mysterious regressions that the cause is we haven't had enough knowledge about what we're doing at this bug. Also, from a positive aspect, if we can get understand what makes our animation also used by other app window is slow, we can even improve the whole Gaia, not only LockScreen.

And I always wonder why your first intuitive idea is to change the window to improve performance. Things change a lot from v1.4 to v2.1, and I can even figure out that to not wait the background app repainting like we've done for master at Bug 1054904 may also improve it significantly. However, I don't mean that thus we should immediately land Bug 1054904, although it's an example to speed up the unlocking without too much possible risks, if we still don't have the information about why your patch works.

Please understand that I don't think your patch is wrong if it's really helpful for performance. In fact, I think it's great and is a chance for us to take a look at our codebase to find out what we can improve for the whole app-window architecture, if we can know the what the patch modified from profiling or directly figure out what's wrong with out code. It would also reduce the possible regressions.

BTW, what I said "profiling" means to use our profiling tool to analyze the function stack and other stuffs. If you don't know how to make the profiling tool work I think Vance may guide you.

Comment 11

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

David, here you can find some information about how to profiling Firefox OS:

https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler#Profiling_Boot_to_Gecko_%28with_a_real_device%29

If you are interested in that, we can discuss in detail off-line

Thanks

Comment 12

[zhaoyang]

Hi Vance and Greg,

   Thank you for the significant suggestion，and it gives me a lot of inspiration.
   
   Thanks a lot

Comment 13

[BMO Automation]

Firefox OS is not being worked on