Closed Bug 917717 Opened 9 years ago Closed 8 years ago

Add memory consumption tests to datazilla

Categories

(Firefox OS Graveyard :: General, defect, P1)

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

RESOLVED FIXED
1.3 C2/1.4 S2(17jan)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: vingtetun, Assigned: hub)

References

Details

(Keywords: perf, Whiteboard: [c=automation p=4 s= u=tarako])

Attachments

(3 files)

Attached patch memory.patchSplinter Review
Looking at the e.me extra ram consumption I build a small xpcom to dump the USS consume directy from JS (navigator.mozMemory.uss).

I would like to add some tests on datazilla to measure the application memory consumption in order to make sure we don't regress and to motivate people to look at it.

Mike can you take the lead with some folks of your team to make this happen?
Flags: needinfo?(mlee)
Maybe more than one :)
Summary: Add some memory consumption test on datazilla → Add some memory consumption tests on datazilla
Dave, can we make use of Vivien's patch to get memory use data on datazilla?

Kyle, any comments on this approach? Would this be helpful to the MemShrink team looking at FxOS memory usage?
Flags: needinfo?(khuey)
Flags: needinfo?(dave.hunt)
Keywords: perf
Whiteboard: [c=automation p= s= u=]
Flags: needinfo?(mlee)
I suspect this would be fairly straightforward to add to b2gperf, just to grab the value after each app launch and submit as a metric to datazilla. That said, we just discussed ultimately migrating the b2gperf tests to make test-perf, so perhaps it should be added directly there?
Flags: needinfo?(dave.hunt)
I don't see how collecting this data could be a negative thing ;-)
Flags: needinfo?(khuey)
It's worth noting that there are lots of different things we could be measuring here though.  Peak memory consumption, average memory consumption, before/after running the app consumption.  Things to think about.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> It's worth noting that there are lots of different things we could be
> measuring here though.  Peak memory consumption, average memory consumption,
> before/after running the app consumption.  Things to think about.

I agree that there is lot of things to measure.

At first I would like to make sure people applications does not consume to much memory at the start of the application. My assumption beeing that memory should be consume only if you need it. 

So it sounds OK if the application consumes more memory when it is in used, but I would like to see if this memory is released afterward as well.
Assignee: nobody → hub
Priority: -- → P1
Whiteboard: [c=automation p= s= u=] → [c=automation p=4 s= u=]
Status: NEW → ASSIGNED
Whiteboard: [c=automation p=4 s= u=] → [c=automation p=4 s= u=][tarako]
I am also hoping to setup a memory consumption metric to understand where things are with tarako (128MB effort)

Ideas such as 
1. getting a snapshot of the available memory (and about_memory) after a complete boot up
2. getting a snapshot of the about_memory before/after launching apps
were discussed 

#2 was raised as I am also working to set up Tarako on the datazilla dashboard to understand the application launch performances. Having about_memory information before/after launching apps seems to be helpful to identify app performance issues 

this is a preliminary thought on how to measure and help define acceptance for Tarako. 
Ideas welcome. 

Thanks
Severity: normal → major
Summary: Add some memory consumption tests on datazilla → Add memory consumption tests to datazilla
Can we scope this down so we can land something before next friday (Dec 20)?  It sounds like for Tarako which is the most urgent need here, that we need memory statistics after app startup and if we can compare that to a baseline idle memory usage, we can get both app startup and app closing memory tests.

Datazilla has a notion that there is one value which is your primary value for tracking in your test. So we could have a startup test and a close down test for each app which would give us both these numbers. And we'd need a third idle test which measures memory usage at the home screen for a period of time to ensure that we have a stable baseline. Once we have these numbers, we can use the Datazilla REST APIs to pull the data out of the system and do any calculations we like. Datazilla also allows us to send in auxiliary data in key value pairs for each observation it takes, so we could include the other items stylesheet memory etc as auxiliary items, but I think we can do that as a v2 of this metric.

What's do-able before Friday?
Flags: needinfo?(mlee)
Flags: needinfo?(jeads)
If as a starting point we could get memory usage after starting with the homescreen running and stabilized, eg just graphs with data showing what we get from b2g-info that would help quite a lot. I don't imagine how we could not do that in a week.

Here's a sample of what needs to be scrapped:
fabrice@fabrice-x230:~/dev/B2G$ b2g-info 
                            |     megabytes     |
           NAME   PID  PPID CPU(s) NICE  USS  PSS  RSS VSIZE OOM_ADJ USER     
            b2g 11751   149   81.0    0 55.5 60.3 73.9 166.9       0 root     
     Homescreen 11819 11751    8.8   18 13.6 18.2 31.4  70.9       8 app_11819
          Usage 11889 11751    2.0   18 11.5 15.7 28.4  67.2      10 app_11889
(Preallocated a 11922 11751    1.3   18  9.2 12.8 24.5  64.7      10 root     

System memory info:

            Total 180.4 MB
     Used - cache 105.0 MB
  B2G procs (PSS) 107.0 MB
    Non-B2G procs  -1.9 MB
     Free + cache  75.4 MB
             Free  19.1 MB
            Cache  56.3 MB
From ctalbert's description this data sounds like it would function like a new test type in the datazilla UI. If this is the case, it will be automatically incorporated into the UI like any other test in datazilla, assuming the JSON submitted is formatted the same way. It would not require new development, if this is not correct, I think I will need more functional specification information to make appropriate changes.
Flags: needinfo?(jeads)
yes, if we can have comment 9 set up as the first step, it will be great
(In reply to Jonathan Eads ( :jeads ) from comment #10)
> From ctalbert's description this data sounds like it would function like a
> new test type in the datazilla UI. If this is the case, it will be
> automatically incorporated into the UI like any other test in datazilla,
> assuming the JSON submitted is formatted the same way. It would not require
> new development, 

i may not be familiar enough with datazilla to visualize it yet but if its feasible, will it be possible to see a prototype if the effort isn't big? thanks
Sadly the patch above has completely rotten..... :-( And to me it looks like we need it before doing anything in make test-perf.
(In reply to Hubert Figuiere [:hub] from comment #13)
> Sadly the patch above has completely rotten..... :-( And to me it looks like
> we need it before doing anything in make test-perf.

Can't you just exec adb shell b2g-info and parse the output?
We could do that, but I was hoping to have some nice interface....
We need that for yesterday. Nice things can wait.
I have parsing for b2g-info in https://github.com/hfiguiere/gaia/tree/bug917717

Hackish but works. I'll plug it in to make test-perf
:hub, mind providing some updates after you come back from the holidays? thanks
Flags: needinfo?(hub)
Since last time I rewrote the code to be synchronous. I need to add this to the test now. 

I have one serious question is that I can't find out how to know the PID or the process name from inside the app in JS (I have marionette to run arbitrary JS in the app). Eventually I could deal with the process UID.

Fabrice do you have an idea?
Flags: needinfo?(hub) → needinfo?(fabrice)
(In reply to Hubert Figuiere [:hub] from comment #19)
> Since last time I rewrote the code to be synchronous. I need to add this to
> the test now. 
> 
> I have one serious question is that I can't find out how to know the PID or
> the process name from inside the app in JS (I have marionette to run
> arbitrary JS in the app). Eventually I could deal with the process UID.
> 
> Fabrice do you have an idea?

If you are in chrome land, you can use js-ctypes to get the pid easily with getpid(). I'm not sure how to get the process name, but reading the source of b2g-info should let you know that (or asking dhylands).
Flags: needinfo?(fabrice)
> If you are in chrome land, you can use js-ctypes to get the pid easily with
> getpid(). I'm not sure how to get the process name, but reading the source
> of b2g-info should let you know that (or asking dhylands).

The pid would be enough, but then I'm not sure I'm at the chrome level when inside marionette.

I was hoping for an API :-/
Where is the marionette code you are working on?
You can access chrome level in Marionette by switching to chrome context.
The problem is that I get the PID of the main process which is uninteresting. I wanted the one of the app :-/
Do you have wip code to show? I don't get what you need and where.
Can you inject a frame script in the child and get it to send you back its pid?
(In reply to Fabrice Desré [:fabrice] from comment #27)
> Can you inject a frame script in the child and get it to send you back its
> pid?

That's part of what I'm trying to solve.
I found a different way. It works now.
Attached file Pull request
Fabrice, is that the kind of data we want?

PR is for gaia. Run |make test-perf|.

It doesn't need the gecko patch above albeit this would be a nice feature. Require running on a device or emulator.,
Attachment #8358022 - Flags: feedback?(fabrice)
So if I run:

|APP=fm make test-perf|

Here is the data I get:

[
{
  "stats": {
    "suites": 1,
    "tests": 1,
    "passes": 1,
    "pending": 0,
    "failures": 0,
    "start": "2014-01-09T22:55:40.889Z",
    "end": "2014-01-09T22:56:56.802Z",
    "duration": 75913,
    "application": "fm"
  },
  "failures": [],
  "passes": [
    {
      "title": "startup time ",
      "fullTitle": "startup test > fm > startup time ",
      "duration": 75591,
      "mozPerfDurations": [
        1839,
        2030,
        1940,
        2004,
        1924
      ],
      "mozPerfDurationsAverage": 1947.4,
      "mozPerfMemory": [
        13.8,
        13.8,
        13.8,
        13.8,
        13.8
      ],
      "mozPerfMemoryAverage": 13.8
    }
  ]
}]
Memory comes in different flavors, so I'd like to get not just a 'mozPerfMemory', but for each app the values of USS  PSS  RSS VSIZE.
And I forgot to mention that we need the system app memory usage too, even if it's not part of the set of the startup time tests.
Fair enough. I'll add that. It should be trivial now.
Here is what I have now:

[
{
  "stats": {
    "suites": 1,
    "tests": 1,
    "passes": 1,
    "pending": 0,
    "failures": 0,
    "start": "2014-01-10T05:02:25.735Z",
    "end": "2014-01-10T05:04:29.152Z",
    "duration": 123417,
    "application": "gallery"
  },
  "failures": [],
  "passes": [
    {
      "title": "startup time ",
      "fullTitle": "startup test > gallery > startup time ",
      "duration": 122649,
      "mozPerfDurations": [
        4767,
        4368,
        4490,
        4718,
        4686
      ],
      "mozPerfDurationsAverage": 4605.8,
      "mozPerfMemory": [
        {
          "app": {
            "uss": 17,
            "pss": 20,
            "rss": 32.4,
            "vsize": 77
          },
          "system": {
            "uss": 65.3,
            "pss": 68.5,
            "rss": 81.1,
            "vsize": 157.4
          }
        },
        {
          "app": {
            "uss": 17,
            "pss": 20,
            "rss": 32.4,
            "vsize": 77
          },
          "system": {
            "uss": 65.3,
            "pss": 68.5,
            "rss": 81.1,
            "vsize": 157.4
          }
        },
        {
          "app": {
            "uss": 17,
            "pss": 20,
            "rss": 32.4,
            "vsize": 77
          },
          "system": {
            "uss": 65.3,
            "pss": 68.5,
            "rss": 81.1,
            "vsize": 157.4
          }
        },
        {
          "app": {
            "uss": 17,
            "pss": 20,
            "rss": 32.4,
            "vsize": 77
          },
          "system": {
            "uss": 65.3,
            "pss": 68.5,
            "rss": 81.1,
            "vsize": 157.4
          }
        },
        {
          "app": {
            "uss": 17,
            "pss": 20,
            "rss": 32.4,
            "vsize": 77
          },
          "system": {
            "uss": 65.3,
            "pss": 68.5,
            "rss": 81.1,
            "vsize": 157.4
          }
        }
      ]
    }
  ]
}]


"app" is for the app, and "system" is for the b2g process.

I don't have the average computation yet.
BTW, who should we have reviewing ? Julien? Vivien? Fabrice?
(In reply to Hubert Figuiere [:hub] from comment #36)
> BTW, who should we have reviewing ? Julien? Vivien? Fabrice?

Probably not me. Also, someone that is available relatively quick...
(In reply to Fabrice Desré [:fabrice] from comment #37)
> (In reply to Hubert Figuiere [:hub] from comment #36)
> > BTW, who should we have reviewing ? Julien? Vivien? Fabrice?
> 
> Probably not me. Also, someone that is available relatively quick...

I can try to get it reviewed by monday.

Hub, with a very quick look I wonder it it works for the Dialer/Contacts app because of entry_points, does it ?
Comment on attachment 8358022 [details] [review]
Pull request

Can you fix the few nits I mentionned and answer 2 questions:
 - Does the code work for entry points? If not can we make it so it works, otherwise it will be useless for 2 important apps (Dialer and Contacts).

 - Does the memory report is runned correctly after the load time is retrieved ? So it won't affects load time on datazilla.

I didn't dive very deep in the algorithm that parsed the b2g-ps output since my understanding is that we will replace it soon and this is just the first step to ramp up.

I know the devtools guys are working on something that may be useful for that. Mike and Panos have been introduced today in order to make sure we don't duplicate work.
Attachment #8358022 - Flags: feedback?(fabrice) → feedback+
Depends on: 958652
Depends on: 920440
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #39)
> Comment on attachment 8358022 [details] [review]
> Pull request
> 
> Can you fix the few nits I mentionned and answer 2 questions:
>  - Does the code work for entry points? If not can we make it so it works,
> otherwise it will be useless for 2 important apps (Dialer and Contacts).

Yes it does work.

> 
>  - Does the memory report is runned correctly after the load time is
> retrieved ? So it won't affects load time on datazilla.

Yes.
Comment on attachment 8358022 [details] [review]
Pull request

Review request, for real. This is a single rebase patch that include the changes from the feedback.
Attachment #8358022 - Flags: review?(21)
Comment on attachment 8358022 [details] [review]
Pull request

There is still a few nits to fix. Please do that before merging.

Again parsing b2g-info is a bad thing but is OK to set up quickly a some graphs and have the ball rolling.

Please file a followup to use an other method and please connect with Panos from the devtools team.
Attachment #8358022 - Flags: review?(21) → review+
Merged

https://github.com/mozilla-b2g/gaia/commit/1fc1be2539b9ed067d9e13f22989e4e1dc7e4dfb

Will file the followup bugs.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Had to remove the exec-sync dependency due to gi bustage.

https://github.com/mozilla-b2g/gaia/commit/c6c2d55efb74e67208311d00e304c93cf1fffedb

Will fix ASAP.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Hubert Figuiere [:hub] from comment #44)
> Had to remove the exec-sync dependency due to gi bustage.
> 
> https://github.com/mozilla-b2g/gaia/commit/
> c6c2d55efb74e67208311d00e304c93cf1fffedb
> 
> Will fix ASAP.

This made Gi on TBPL green again, thanks.
Now we need to debug it.

https://tbpl.mozilla.org/php/getParsedLog.php?id=32930299&full=1&branch=b2g-inbound

In the full log above, the puzzling error is "11:06:24     INFO -  make[1]: g++: Command not found".

*sigh*
Status: REOPENED → ASSIGNED
In buildbot, we need to use pre-built packages for node modules that need compilation.  We've had to do the same for gaia-integration tests.  Gareth should be able to help you with what to do.
Gareth, can you help us getting the exec-sync NodeJS module in? Thanks.
Flags: needinfo?(gaye)
Hi Hub,

You need to pre-compile exec-sync on ubuntu 32-bit && 64-bit vms. Also you should fork it and add a tiny hack sockit-to-me does where npm install checks for g++ and fails over to copying the precompiled binaries to build/Release.

Or you could use regular node exec...
Flags: needinfo?(gaye) → needinfo?(hub)
Regular node exec is async which is not what I want. Been there, done that. I need something sync.

I don't have Ubuntu. Do we have a VM somewhere I could get an account on to do it?
Flags: needinfo?(hub)
Flags: needinfo?(gaye)
I just used a personal ec2 account to precompile sockit-to-me (this might be helpful https://github.com/mozilla-b2g/sockit-to-me/blob/master/tools/prebuild.sh). Also #releng may be able to help you. It would be really nice to automate some of this too... sorry!
Flags: needinfo?(gaye)
I don't have ec2 accounts either. I'll ask releng.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Depends on: 959629
mistakenly closed
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
(In reply to Hubert Figuiere [:hub] from comment #52)
> I don't have ec2 accounts either. I'll ask releng.

You can file a releng bug to request a loaner of an EC2 slave; file under Release Engineering:Loan Requests.
I actually file bug 959629 to request the module. We'll see.
Flags: needinfo?(mlee)
Whiteboard: [c=automation p=4 s= u=][tarako] → [c=automation p=4 s= u=tarako] [tarako]
Blocks: 920440
No longer depends on: 920440
Attached file Part 2: Pull request
Gareth, here is the pull request for the fix.

Module has been published but still doesn't have the prebuilt stuff as I'm blocked on IT.
Attachment #8361821 - Flags: review?(gaye)
Now all we needs is this patch in now.
Comment on attachment 8361821 [details] [review]
Part 2: Pull request

LGTM. One nit on GH.
Attachment #8361821 - Flags: review?(gaye) → review+
Merged.

https://github.com/mozilla-b2g/gaia/commit/4c29941ae4adf800bac335e33e11a30a602c0e4a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Blocks: 962238
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
triage: 1.3T to enable this on tarako builds
blocking-b2g: --- → 1.3T+
Whiteboard: [c=automation p=4 s= u=tarako] [tarako] → [c=automation p=4 s= u=tarako]
I'm reopening this because although the test jobs are running successfully in Jenkins, we're only reporting app startup times to datazilla, and not the memory statistics.

I think we just need to modify https://github.com/mozilla/b2gperf/blob/master/b2gperf/mozperf.py to parse the memory statistics out of the JSON and submit them to datazilla.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Nm, this is handled by bug 962238.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Hi Ying Xu, heard that you will be doing uplifts to 1.3T branch. After you completed the uplift, can you please set status-b2g-v1.3T to fixed? please let us know if you have problems with it. thanks
Flags: needinfo?(ying.xu)
(In reply to Joe Cheng [:jcheng] from comment #63)
> Hi Ying Xu, heard that you will be doing uplifts to 1.3T branch. After you
> completed the uplift, can you please set status-b2g-v1.3T to fixed? please
> let us know if you have problems with it. thanks
 
Have confilicts.Please help to resolve it.Thank you.
Flags: needinfo?(jcheng)
Flags: needinfo?(jcheng) → needinfo?(hub)
In automation we use gaia master to run the tests against 1.3t. So there is no need to uplift this, which is likely not easy as there are other things that have never been uplifted to the 1.3 branch and thus aren't in 1.3t.
Flags: needinfo?(hub)
wontfix the 1.3t branch then.
Flags: needinfo?(ying.xu)
See Also: → 1044297
You need to log in before you can comment on or make changes to this bug.