Closed
Bug 972308
Opened 11 years ago
Closed 11 years ago
[DIALER]slow response when click on dialer panel
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect, P2)
Tracking
(blocking-b2g:1.3T+, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 unaffected, b2g-v1.3 wontfix, b2g-v1.3T verified, b2g-v1.4 fixed, b2g-v2.0 fixed)
Tracking | Status | |
---|---|---|
b2g18 | --- | unaffected |
b2g-v1.1hd | --- | unaffected |
b2g-v1.2 | --- | unaffected |
b2g-v1.3 | --- | wontfix |
b2g-v1.3T | --- | verified |
b2g-v1.4 | --- | fixed |
b2g-v2.0 | --- | fixed |
People
(Reporter: ying.xu, Assigned: ying.xu)
References
Details
(Keywords: perf, Whiteboard: [c=progress p= s= u=])
Attachments
(3 files, 2 obsolete files)
device tarako v1.3T branch
reproduce way:
open dialer app, click on figures of dialer panel
response is very slow
we can confirm that the tone is not the reason for this through the DS5 profiling result.
need to investigate this
blocking-b2g: --- → 1.3T?
Summary: [DIALER]slow response when click on dialer panel → [tarako][DIALER]slow response when click on dialer panel
Target Milestone: --- → 1.4 S1 (14feb)
User 86%, System 13%, IOW 0%, IRQ 0%
User 84 + Nice 183 + Sys 40 + Idle 0 + IOW 0 + IRQ 0 + SIRQ 0 = 307
PID PR CPU% S #THR VSS RSS PCY UID Name
1812 0 56% R 18 79196K 26872K fg app_1812 /system/b2g/plugin-container
81 0 31% S 51 156248K 35936K fg root /system/b2g/b2g
Comment 3•11 years ago
|
||
Was this tested before or after bug 917193 landed? That bug should have improved the situation here IMHO.
(In reply to Gabriele Svelto [:gsvelto] from comment #3)
> Was this tested before or after bug 917193 landed? That bug should have
> improved the situation here IMHO.
bug 917193 has improved the situation actually, but not fix this.
And I noticed that deleting a figure by clicking on the 'X' button is very quick.
Updated•11 years ago
|
Updated•11 years ago
|
Whiteboard: [c= p= s= u=tarako] → [c=progress p= s= u=tarako]
Comment 6•11 years ago
|
||
I think bug 967440 can help here. It's not in my list of priorities though so if anyone wants to finish it, feel free to steal it from me.
Updated•11 years ago
|
Whiteboard: [c=progress p= s= u=tarako] → [c=progress p= s= u=tarako] [SI testing blocker]
Updated•11 years ago
|
Whiteboard: [c=progress p= s= u=tarako] [SI testing blocker] → [c=progress p= s= u=tarako] [SI-testing-blocker]
Updated•11 years ago
|
Component: Gaia → Gaia::Dialer
Comment 7•11 years ago
|
||
QAWANTED to confirm latest status. thanks
Flags: needinfo?(brhuang)
Keywords: qawanted
Comment 8•11 years ago
|
||
William, please help on this confirmation. If the keyboard response is acceptable, upload a video for partner.
Flags: needinfo?(brhuang) → needinfo?(whsu)
Comment 9•11 years ago
|
||
Hi, Brian,
No problem. Thanks for the reminder.
-----------------------------------------------------------
Hi, all,
After I did the performance measurement, I think the current keypad response time of Tarako device is acceptable.
The keypad response time of Taroko device is 0.2362 second/key. (Build ID: 20140310)
The keypad response time of Buri device is 0.2124 second/key.
Compare with Buri device, the keypad response time of Tarako only increases 0.0238 second/key
My test scenario is as below.
Step 1. Launch the dialer app
Step 2. Try to input 10 numbers.
Step 3: Repeat 5 times
Here are the demo video.
Buri: https://dc1.safesync.com/LMtzcYMp/Buri_Dialer_20140310.wmv?a=WYfDsc0_NKc
Tarako: https://dc1.safesync.com/LMtzcYMp/Tarako_Dialer_20140310.wmv?a=74Stj9qirmo
Also, attach the test result and raw data. (Dialer_keypad_20140310.png)
Buri:
01. 2.17 seconds (10 keys)
02. 2.13 seconds (10 keys)
03. 2.11 seconds (10 keys)
04. 2.07 seconds (10 keys)
05. 2.14 seconds (10 keys)
Tarako:
01. 2.36 seconds (10 keys)
02. 2.31 seconds (10 keys)
03. 2.26 seconds (10 keys)
04. 2.53 seconds (10 keys)
05. 2.35 seconds (10 keys)
* Build Info.:
Buri:
- Gaia 71f78f7f68fcf5e23cc5965fee0dad45289c438b
- Gecko https://hg.mozilla.org/mozilla-central/rev/5dc091b7e86f
- BuildID 20140304160205
- Version 30.0a1
Tarako:
- Gaia 7f434a8799b8c7b2b419ea118b2de7c2682c170d
- Gecko 8da7b6293d63b140932893fed6a45f91bfa93631
- BuildID 20140310075556
- Version 28.0
------------------------------------------------------------------
So, I think we can close this bug.
Any thought? Thanks.
Flags: needinfo?(whsu)
Comment 10•11 years ago
|
||
Comment 11•11 years ago
|
||
James, seem like we can resolve this bug as workforme, can you confirm on your side? thanks
Flags: needinfo?(james.zhang)
Comment 12•11 years ago
|
||
(In reply to Joe Cheng [:jcheng] from comment #11)
> James, seem like we can resolve this bug as workforme, can you confirm on
> your side? thanks
If close audio, it's still slow.
Please compare it with android dialer.
Flags: needinfo?(james.zhang)
Comment 13•11 years ago
|
||
Hi, James,
Here are my demo video.
- https://dc1.safesync.com/LMtzcYMp/Tarako_Dialer_20140310.wmv?a=74Stj9qirmo
I didn't see heavy delay while I did the test on the latest Tarako build.
No matter the keypad sound was turned on/off, the keypad worked as usual.
Could you please provide the 1) Build ID 2) Commit number?
Also, if possible, please provide the keypad response time of android platform.
Thanks for your help.
Updated•11 years ago
|
Flags: needinfo?(james.zhang)
Comment 14•11 years ago
|
||
(In reply to William Hsu [:whsu] from comment #13)
> Hi, James,
>
> Here are my demo video.
> - https://dc1.safesync.com/LMtzcYMp/Tarako_Dialer_20140310.wmv?a=74Stj9qirmo
>
> I didn't see heavy delay while I did the test on the latest Tarako build.
> No matter the keypad sound was turned on/off, the keypad worked as usual.
>
> Could you please provide the 1) Build ID 2) Commit number?
>
> Also, if possible, please provide the keypad response time of android
> platform.
> Thanks for your help.
Can you get android pac from kai-zhen.li and compare with your version?
Ying, do we need provide tarako DS5 profiling result?
Flags: needinfo?(james.zhang) → needinfo?(ying.xu)
Comment 15•11 years ago
|
||
back to 1.3T? as we are waiting further information
blocking-b2g: 1.3T+ → 1.3T?
Comment 16•11 years ago
|
||
triage: minus for now. please nominate again if this bug still happens
blocking-b2g: 1.3T? → -
Whiteboard: [c=progress p= s= u=tarako] [SI-testing-blocker] → [c=progress p= s= u=tarako]
Assignee | ||
Comment 17•11 years ago
|
||
After I disable the audio when clicking on the touch,
The response is highly improved.
diff --git a/apps/communications/dialer/js/tone_player.js b/apps/communications/dialer/js/tone_player.js
index b62bc89..ccefd62 100644
--- a/apps/communications/dialer/js/tone_player.js
+++ b/apps/communications/dialer/js/tone_player.js
@@ -131,9 +131,9 @@ var TonePlayer = {
start: function tp_start(frequencies, shortPress) {
if (shortPress) {
- this._playSample(frequencies);
+ // this._playSample(frequencies);
} else {
- this._startAt(frequencies, 0, shortPress ? kShortPressDuration : 0);
+ // this._startAt(frequencies, 0, shortPress ? kShortPressDuration : 0);
}
},
tested by top
User 86%, System 10%, IOW 0%, IRQ 0%
User 272 + Nice 0 + Sys 32 + Idle 9 + IOW 0 + IRQ 0 + SIRQ 0 = 313
PID PR CPU% S #THR VSS RSS PCY UID Name
22232 0 85% R 47 139900K 48012K fg root /system/b2g/b2g
Flags: needinfo?(ying.xu)
Assignee | ||
Comment 18•11 years ago
|
||
another try
This patch also improved a lot.
Seems new audio() cuased audio driver in HAL and kernel took much CPU time.
diff --git a/apps/communications/dialer/js/tone_player.js b/apps/communications/dialer/js/tone_player.js
index b62bc89..ceb4aff 100644
--- a/apps/communications/dialer/js/tone_player.js
+++ b/apps/communications/dialer/js/tone_player.js
@@ -9,6 +9,7 @@ const kReleaseDuration = 0.05;
var TonePlayer = {
_audioContext: null,
+ _audioElement: null,
_gainNode: null,
_playingNodes: [],
_tonesSamples: {
@@ -51,12 +52,16 @@ var TonePlayer = {
*/
_playSample: function tp_playSample(frequencies) {
var sample = null;
+ if(!this._audioElement)
+ this._audioElement = new Audio();
+
+ sample = this._audioElement;
for (var i in this._tonesSamples) {
if ((frequencies.length === 2) &&
(frequencies[0] === this._tonesSamples[i][0]) &&
- (frequencies[1] === this._tonesSamples[i][1])) {
- sample = new Audio(i);
+ (frequencies[1] === this._tonesSamples[i][1])) {
+ sample.src = i;
break;
}
}
Updated•11 years ago
|
Summary: [tarako][DIALER]slow response when click on dialer panel → [DIALER]slow response when click on dialer panel
Comment 19•11 years ago
|
||
Ying, did you want to submit an official patch for review?
Severity: critical → normal
blocking-b2g: - → backlog
Flags: needinfo?(ying.xu)
Priority: P1 → P2
Whiteboard: [c=progress p= s= u=tarako] → [c=progress p= s= u=]
Target Milestone: 1.4 S1 (14feb) → ---
Assignee | ||
Comment 20•11 years ago
|
||
Attachment #8392000 -
Flags: review?(etienne)
Flags: needinfo?(ying.xu)
Assignee | ||
Comment 21•11 years ago
|
||
The audio is still missing sometimes.
We know some problems in our audio drivers.We will fix that.
Comment 22•11 years ago
|
||
Comment on attachment 8392000 [details]
PR to review , based on v1.3t
Looking good but forwarding the review to Gabriele who did most of the performance work on this part of the dialer.
Also please note that we'll need to land the patch on master first.
Attachment #8392000 -
Flags: review?(gsvelto)
Attachment #8392000 -
Flags: review?(etienne)
Attachment #8392000 -
Flags: feedback+
Comment 23•11 years ago
|
||
Comment on attachment 8392000 [details]
PR to review , based on v1.3t
I've added a couple of nits on the GitHub PR but there's a fundamental issue with this patch that keeps me from r+'ing it and that I don't think it's fixable.
Whenever you set the |src| property it seems to me that the previous sound finishes playing and then the new one is played; this means that when typing really fast we skip some sounds. The samples are 120ms in length IIRC so this isn't a big issue and we might just live with it until we switch back to Web Audio proper, but I'd like to hear what :etienne thinks about this before r+'ing your patch.
Also you should provide a patch/PR against the master branch first, once we have that reviewed and landed we can uplift it here.
Flags: needinfo?(etienne)
Comment 24•11 years ago
|
||
Not directly answering the question but: wouldn't bug 967440 help us a lot here?
If we can get the best user experience with acceptable cpu usage that's a win :)
Flags: needinfo?(etienne)
Comment 25•11 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #24)
> Not directly answering the question but: wouldn't bug 967440 help us a lot here?
> If we can get the best user experience with acceptable cpu usage that's a win :)
It might, I'll try to give it a spin tomorrow on my Tarako and see if it helps. We've already got a workaround for this particular issue and the idea of having a second workaround on top of the first doesn't make me happy :-|
Assignee | ||
Comment 26•11 years ago
|
||
Attachment #8392640 -
Flags: review?(etienne)
Assignee | ||
Comment 27•11 years ago
|
||
Attachment #8392000 -
Attachment is obsolete: true
Attachment #8392000 -
Flags: review?(gsvelto)
Attachment #8392641 -
Flags: review?(etienne)
Comment 28•11 years ago
|
||
Comment on attachment 8392640 [details]
PR to review @master
forwarding to Gabriele and carrying my feedback+ since the gain in noticeable (a lot less key highlight loss etc...).
Attachment #8392640 -
Flags: review?(gsvelto)
Attachment #8392640 -
Flags: review?(etienne)
Attachment #8392640 -
Flags: feedback+
Updated•11 years ago
|
Attachment #8392641 -
Flags: review?(etienne) → review?(gsvelto)
Comment 29•11 years ago
|
||
Comment on attachment 8392640 [details]
PR to review @master
r=me with the nits addressed, see my comment on GitHub.
Attachment #8392640 -
Flags: review?(gsvelto) → review+
Comment 30•11 years ago
|
||
Comment on attachment 8392641 [details]
PR to review @v1.3t
The patch is identical to the master one so let's cherry pick the change once we've landed it on master; we don't need a separate pull request for this.
Attachment #8392641 -
Attachment is obsolete: true
Attachment #8392641 -
Flags: review?(gsvelto)
Updated•11 years ago
|
Assignee: nobody → ying.xu
Status: NEW → ASSIGNED
Assignee | ||
Comment 31•11 years ago
|
||
update according to Comment 29 Gabriele Svelto [:gsvelto] 2014-03-19 08:39:27 PDT
Attachment #8393959 -
Flags: review?(gsvelto)
Comment 32•11 years ago
|
||
Comment on attachment 8393959 [details]
PR to review@master
I had to restart the marionette_js test job and now Travis is green, ready to merge. Note that you don't need to ask for review again if I have already given you an r+ with nits, you just have to address the nits.
Attachment #8393959 -
Flags: review?(gsvelto) → review+
Comment 33•11 years ago
|
||
Merged to gaia/master 75cc0092a71d4a4e8ee2c81200ed60cf810f05e2
Nom'ing again as this might be needed for decent keypad tones on Tarako.
Status: ASSIGNED → RESOLVED
blocking-b2g: backlog → 1.3T?
Closed: 11 years ago
status-b2g18:
--- → unaffected
status-b2g-v1.1hd:
--- → unaffected
status-b2g-v1.2:
--- → unaffected
status-b2g-v1.3:
--- → affected
status-b2g-v1.3T:
--- → affected
status-b2g-v1.4:
--- → affected
status-b2g-v2.0:
--- → fixed
Resolution: --- → FIXED
Comment 34•11 years ago
|
||
hi YingXu, how much improvement do you see here? lets understand the improvement here before we decide for tarako. thanks
Flags: needinfo?(ying.xu)
Assignee | ||
Comment 35•11 years ago
|
||
(In reply to Joe Cheng [:jcheng] from comment #34)
> hi YingXu, how much improvement do you see here? lets understand the
> improvement here before we decide for tarako. thanks
very obvious improvement.
While the side effect is obvious, some of click tone will be missing if press the button very fast
Flags: needinfo?(ying.xu)
Comment 36•11 years ago
|
||
triage: 1.3T+ to improve performance. can you please update it and resolved fixed on status-b2g-v1.3T? thanks
blocking-b2g: 1.3T? → 1.3T+
Comment 37•11 years ago
|
||
Why has this patch not been uplifted yet?
Assignee | ||
Comment 38•11 years ago
|
||
Updated•11 years ago
|
Target Milestone: --- → 1.4 S4 (28mar)
Comment 39•11 years ago
|
||
Following STR from Comment 0, clicking on figures on keypad is in the acceptable range and doesn't give the perception of being slow on the latest 1.3T build.
Environmental Variables:
Device: Tarako 1.3T MOZ
BuildID: 20140428014001
Gaia: 8895b180ed636069473703d0e7b73086989601ce
Gecko: 7caf4b5abfce
Version: 28.1
Firmware Version: sp6821a_gonk4.0_user.pac
Comment 40•10 years ago
|
||
Comment on attachment 8393959 [details]
PR to review@master
Request for uplift from partner for Dolphin 1.4.
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Initial implementation
[User impact] if declined: Dialer sound is off on slow devices
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Small change, just caching the audio element
[String changes made]: None
Attachment #8393959 -
Flags: approval-gaia-v1.4?
Comment 42•10 years ago
|
||
Comment on attachment 8393959 [details]
PR to review@master
will review in triage
Attachment #8393959 -
Flags: approval-gaia-v1.4?
Comment 43•10 years ago
|
||
Comment on attachment 8393959 [details]
PR to review@master
taking in 1.4
Attachment #8393959 -
Flags: approval-gaia-v1.4+
Updated•10 years ago
|
blocking-b2g: 1.4? → 1.3T+
Comment 44•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•