Filed by: wkocher [at] mozilla.com https://treeherder.mozilla.org/logviewer.html#?job_id=34643481&repo=mozilla-inbound https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-linux64/1472133506/mozilla-inbound_ubuntu64_vm_test-xpcshell-bm121-tests1-linux64-build72.txt.gz

Seems like we need to update the test or slim down on memory. :ochameau, what do you think?

Personally, I think this test is not carrying its weight. We don't have any way to identify which changes are increasing memory footprint, just some unlucky stiff who gets blamed when they land the straw that breaks the camels back. We should replace this with some kind of perfherder/DAMP thing that can actually track regressions down to the push granularity.

The test doesn't catch small regressions over time (like <5%), which talos doesn't do either. But definitely catch big mistakes. Having said that I also dislike the fact that it blames random platform developers which just trigger the limit. Unfortunately, I don't think DAMP could probe the memory. I imagine it can only measure one precise probe which is the time to load a toolbox under various conditions. Brian, Could you tell us if we can probe the memory usage in DAMP or would it require yet another talos task?

If tweaking DAMP to measure memory is hard, just bump that threshold. To something high enough to work also on linux: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b0e358855e4&selectedJob=26667022

(In reply to Alexandre Poirot [:ochameau] from comment #4) > The test doesn't catch small regressions over time (like <5%), which talos > doesn't do either. But definitely catch big mistakes. Talos does catch small regressions, we just choose not to act on them usually > Having said that I also dislike the fact that it blames random platform > developers which just trigger the limit. Unfortunately, I don't think DAMP > could probe the memory. I imagine it can only measure one precise probe > which is the time to load a toolbox under various conditions. > > Brian, Could you tell us if we can probe the memory usage in DAMP or would > it require yet another talos task? There's an option that might be even easier. If you export a string in the logs called PERFHERDER_DATA you can get perfherder tracking on any kind of data from any test: http://wrla.ch/blog/2015/11/perfherder-onward/. Then rather than setting a limit that causes the test to fail, we can track this over time and get alerts as it changes. Here's an example test that does that: https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/chrome/test_awsy_lite.html. I think this might be a good case for using this feature instead of talos. If we decide we'd rather fold this into talos: we can add a probe for any kind of number so I don't see why not (assuming that we can measure the memory in an addon + e10s context). We would need to normalize it so that the measurement had the right kind of weighting along with other probes (which are in ms). So we would have a couple of choices: 1. Make the number very small relative to other probes. In this case it would never cause alerts but we can manually track it and graph it over time 2. Make the number about the same relative to other probes so that it would cause an alert if the number jumped up

(In reply to Brian Grinstead [:bgrins] from comment #7) > There's an option that might be even easier. If you export a string in the > logs called PERFHERDER_DATA you can get perfherder tracking on any kind of > data from any test: http://wrla.ch/blog/2015/11/perfherder-onward/. Oh! I missed that, I will give that a try. Definitely easier than working around Talos!

(In reply to Alexandre Poirot [:ochameau] from comment #8) > (In reply to Brian Grinstead [:bgrins] from comment #7) > > There's an option that might be even easier. If you export a string in the > > logs called PERFHERDER_DATA you can get perfherder tracking on any kind of > > data from any test: http://wrla.ch/blog/2015/11/perfherder-onward/. > > Oh! I missed that, I will give that a try. Definitely easier than working > around Talos! Yeah, if you do this use the "awsy" framework. We're already tracking are-we-slim-yet and android memory measurements with that framework, so this would be a natural fit: https://treeherder.mozilla.org/perf.html#/alerts?status=0&framework=4&page=1

Hum. I tried to emit PERFHERDER_DATA from xpcshell, but I don't think it will work on treeherder as we don't see output of tests on treeherder for xpcshell tests. Only test final result: 10:28:23 INFO - TEST-PASS | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_compatoverrides.js | took 11480ms 10:28:23 INFO - TEST-START | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_corruptfile.js 10:28:29 INFO - TEST-PASS | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_corrupt.js | took 16194ms ... I'll try to come up with a mochitest, but memory is much more fuzzy there as we load Firefox and various random things. May be mochitest-chrome would be more stable.

Does it work if you print out the strings with `do_print`?

As long as PERFHERDER_DATA is in the log file (see talos jobs for examples), Treeherder/Perfherder should pick it up.

It wasn't easy to convert it to mochitest because of the noise... I had to use the horrible processNextEvents to have something stable locally, hopefully it will be stable on build machines as well!

xpcshell is a dead end as test output is only shown when a test fail. There is no equivalent of SimpleTest.requestCompleteLog() on xpcshell.

Comment on attachment 8787184 [details] Bug 1298113 - Convert devtools xpcshell memory assertion into a mochitest posting data to perfherder. Here is a try build of it. Do not hesitate to f?/r? any additional peer! It seems to work fine. I'm wondering if we can see the intermediate values in perherder? It looks like we can only see the final one, which is not handy in this test which track multiple steps.

hum. try just got the results for many runs and it looks unstable going from 2M to 5M. My forceGC+processNextEvent trick doesn't seem to be enough. I suspect something from Firefox is running while the test is executed and allocate a bunch of random things...

Comment on attachment 8787184 [details] Bug 1298113 - Convert devtools xpcshell memory assertion into a mochitest posting data to perfherder. https://reviewboard.mozilla.org/r/76024/#review74566 Looks like a nice approach to me. Hopefully we can stabilize the values enough to make this workable.

Comment on attachment 8786730 [details] [diff] [review] Bump DebuggerServer.init memory threshold in test. I'm unable to stabilize the memory probe in mochitest, so I think I will just bump the existing xpcshell threshold for now...

Comment on attachment 8787184 [details] Bug 1298113 - Convert devtools xpcshell memory assertion into a mochitest posting data to perfherder. https://reviewboard.mozilla.org/r/76024/#review75066 ::: devtools/server/tests/browser/browser_memory-footprint.js:77 (Diff revision 2) > + let footprint = (gMgr.residentUnique - refMemory) / 1024; > + let PERFHERDER_DATA = { > + framework: { name: "awsy" }, > + suites: [{ > + subtests, > + name: "Devtools memory usage", value: footprint IMO it's a bit confusing the way that the subtests for this correspond to memory usage after different actions are taken. Unless there's a big benefit to measuring these, I'd probably just check memory usage after listTabs is called.

NI'ing Eric, who might have some insight on how to stabilize the memory numbers. Eric: see comment 21. If we can't figure out how to make the numbers stable from mochitest, maybe we should consider adding the capability of outputting perfherder data from an xpcshell test. I suspect this won't be the only case where we might want to do that.

(In reply to William Lachance (:wlach) from comment #29) > NI'ing Eric, who might have some insight on how to stabilize the memory > numbers. Eric: see comment 21. > > If we can't figure out how to make the numbers stable from mochitest, maybe > we should consider adding the capability of outputting perfherder data from > an xpcshell test. I suspect this won't be the only case where we might want > to do that. If you want to try the mochitest route (we effectively do this w/AWSY) you probably want to do something like: - Load test, wait 30s - Force GC [1], maybe wait some more - Get your baseline memory measurement - Do your thing, wait 30s - Force GC, maybe wait some more - Get your memory-used-by-devtools measurement If someone wants to implement it, I'd be okay with adding an additional AWSY test (as in, in the AWSY suite) for this. [1] https://github.com/mozilla/areweslimyet/blob/master/benchtester/test_memory_usage.py#L190-L200

Comment on attachment 8786730 [details] [diff] [review] Bump DebuggerServer.init memory threshold in test. Review of attachment 8786730 [details] [diff] [review]: ----------------------------------------------------------------- Shouldn't we remove this test instead of bumping the level? It seems like we've agreed there's not much value in this existing test...?

Looks unstable locally and report negative numbers. https://treeherder.mozilla.org/#/jobs?repo=try&revision=895a450190a3

Ok, here is just the removal of the xpcshell test.

Comment on attachment 8788965 [details] [diff] [review] Remove devtools xpcshell test asserting for memory consumption Review of attachment 8788965 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! Please either file a new bug or mark this one leave-open so we can continue working out what the new approach should be (perfherder from mochitest, awsy, etc.).

Eric, See comment 33 and comment 34. Two new patches, with 30s pauses before and after, but it looks like there is still significant things being done after 30s. Or there is something special about devtools... I end up with even more memory being freed. I tried various very hacky things without any luck. xpcshell would be easier as we better control what is being loaded, but it sounds as complex to change the test harness output :/

https://hg.mozilla.org/integration/fx-team/rev/054d2453311b5adf511c5cfaa59e27db955405f7 Bug 1298113 - Remove devtools xpcshell test asserting for memory consumption. r=jryans

(In reply to Alexandre Poirot [:ochameau] from comment #38) > Eric, See comment 33 and comment 34. Two new patches, with 30s pauses before > and after, but it looks like there is still significant things being done > after 30s. Or there is something special about devtools... I end up with > even more memory being freed. I tried various very hacky things without any > luck. > > xpcshell would be easier as we better control what is being loaded, but it > sounds as complex to change the test harness output :/ Sorry for the delay here, yes I think xpcshell is the best way to go if you want consistent numbers.

(In reply to Eric Rahm [:erahm] from comment #42) > (In reply to Alexandre Poirot [:ochameau] from comment #38) > > Eric, See comment 33 and comment 34. Two new patches, with 30s pauses before > > and after, but it looks like there is still significant things being done > > after 30s. Or there is something special about devtools... I end up with > > even more memory being freed. I tried various very hacky things without any > > luck. > > > > xpcshell would be easier as we better control what is being loaded, but it > > sounds as complex to change the test harness output :/ > > Sorry for the delay here, yes I think xpcshell is the best way to go if you > want consistent numbers. I was expecting something from you about xpcshell support on perfherder. I don't have the bandwidth to look at that. Are you planning to support it anytime soon? Or is there anyone I could ping about that? The issue being that xpcshell test suite doesn't print anything unless a test fail.

(In reply to Alexandre Poirot [:ochameau] from comment #43) > (In reply to Eric Rahm [:erahm] from comment #42) > > (In reply to Alexandre Poirot [:ochameau] from comment #38) > > > Eric, See comment 33 and comment 34. Two new patches, with 30s pauses before > > > and after, but it looks like there is still significant things being done > > > after 30s. Or there is something special about devtools... I end up with > > > even more memory being freed. I tried various very hacky things without any > > > luck. > > > > > > xpcshell would be easier as we better control what is being loaded, but it > > > sounds as complex to change the test harness output :/ > > > > Sorry for the delay here, yes I think xpcshell is the best way to go if you > > want consistent numbers. > > I was expecting something from you about xpcshell support on perfherder. > I don't have the bandwidth to look at that. Are you planning to support it > anytime soon? > Or is there anyone I could ping about that? > The issue being that xpcshell test suite doesn't print anything unless a > test fail. Ah okay, that's maybe a wlach question.

(In reply to Eric Rahm [:erahm] from comment #44) > (In reply to Alexandre Poirot [:ochameau] from comment #43) > > (In reply to Eric Rahm [:erahm] from comment #42) > > > (In reply to Alexandre Poirot [:ochameau] from comment #38) > > > > Eric, See comment 33 and comment 34. Two new patches, with 30s pauses before > > > > and after, but it looks like there is still significant things being done > > > > after 30s. Or there is something special about devtools... I end up with > > > > even more memory being freed. I tried various very hacky things without any > > > > luck. > > > > > > > > xpcshell would be easier as we better control what is being loaded, but it > > > > sounds as complex to change the test harness output :/ > > > > > > Sorry for the delay here, yes I think xpcshell is the best way to go if you > > > want consistent numbers. > > > > I was expecting something from you about xpcshell support on perfherder. > > I don't have the bandwidth to look at that. Are you planning to support it > > anytime soon? > > Or is there anyone I could ping about that? > > The issue being that xpcshell test suite doesn't print anything unless a > > test fail. > > Ah okay, that's maybe a wlach question. Actually I don't really know either. :( jgriffin, who on our team would be best able to answer how hard it would be to get xpcshell to print out something to standard out in the case that tests don't fail?

Ahal, can you answer comment #45?

They do print stuff on test pass already. Adding additional logging to stdout should be as easy as calling self.log.info(...). The problem with xpcshell is that each test is run in parallel in a separate process, so TEST-START/TEST-END messages don't come one after another. We could potentially create a new formatter that displays output better if necessary.

Alex, any interest in reviving this work? This would be a good path to tracking DevTools per-process memory usage if we can get it working. For xpcshell, what about trying to force the test in sequential mode via the `run-sequentially`[1] manifest flag? Perhaps that will force the log to appear for Perfherder data? [1]: https://searchfox.org/mozilla-central/search?q=sequentially&path=xpcshell

The xpcshell test was already using run-sequentially to have less noise in memory measurement. It looks like it doesn't change xpcshell stdout behavior: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bcc7295ec50cb90d7d216394793be061ce0d366&selectedJob=170326419 https://taskcluster-artifacts.net/BcBcISgSQdC6kem-qM5elg/0/public/logs/live_backing.log (look for test_memory.js) I tried various ways to print something to stdout, but nothing is displayed. I think everything is blocked from the python app running xpcshell tests.

We may use DAMP to record memory consumption, but I'm afraid we would have same issue as mochitests where the measurement has a lot of variance.