New issue
Advanced search Search tips

Issue 833892 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression
Proj-XR



Sign in to add a comment

2.7%-4% regression in xr.browsing.static at 550480:550532

Project Member Reported by bsheedy@google.com, Apr 17 2018

Issue description

Culprit is https://chromium-review.googlesource.com/994129. Th increase is pretty small and seems expected. If this is the case, just close as a wontfix.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Apr 17 2018

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=833892

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=b972d14df6b9a2e8e6ef5f90d7aec14f6413a490d273c7e28d1b0d6baf21c5ce


Bot(s) for this bug's original alert(s):

pixel_xl
Components: Internals>VR
Labels: Proj-VR VR-Perf
Maybe?

I added two strings. On a desktop machine I'd expect that to reserve < 50 bytes. It looks like the jump here is > 200 bytes. The reserved capacity for strings on the platform being tested may be larger (or the strings (urls) maybe longer).

So this delta is in the ballpark, but at the high end of that ballpark.
Owner: bsheedy@chromium.org
bsheedy@ do you know what the default string space is on this platform?

e.g.
std::string foo;
LOG(INFO)<<"The default reserved space for a string is "<<foo.capacity();

If it's 100 bytes or so, this lines up (the regression lines up with the change), i.e. WontFix.

Or if the chrome flag omnibox-rich-entity-suggestions is enabled then the string size makes sense, i.e. WontFix.
Just tested, the default capacity on Android is 10 bytes.

This isn't the exact same environment as the tests were run on (I ran on a debug build with Android O while the perf tests are run on a release build with Android N), but I don't think that should make a difference?
Owner: dschuyler@chromium.org
Owner: bsheedy@chromium.org
bsheedy@, It sounds like something unexpected is happening.

I can look at the code and provide suggestions (as I've done above), but I've not built for Android (or VR?). Can someone more familiar with the platform and build environment check on this -- or are there guides I could follow to build on this platform and try to diagnose it (will I need any special hardware)?

Would it be easy to log the values of the two strings (image_dominant_color and image_url) to see what the values are? 
Cc: tiborg@chromium.org
Owner: dschuyler@chromium.org
I added logging to all the places where image_dominant_color and image_url are set in autocomplete_match.cc, base_search_provider.cc, and search_suggestion_parser.cc, but none of those log statements are ever reached. I think that's expected since the perf tests in question don't interact with the omnibox at all, so there shouldn't be any suggestions/autocomplete. However, that still leaves the question of where the extra memory usage is coming from.

It looks like this isn't actually VR-related, as the same increase occurred in the normal memory tests https://chromeperf.appspot.com/report?sid=0d9822452ba15c04f0d44b773bdabeec8b65d2f2fd3ca79bc9a76c4a60ab4b13. It just looks like it was never alerted on.

Running the VR versions is fairly involved, but running the regular memory benchmark should be pretty simple. Build the telemetry_perf_tests target with the following gn args:

ffmpeg_branding = "Chrome"
goma_dir = "/b/build/slave/cache/goma_client"
is_chrome_branded = true
is_debug = false
is_official_build = true
proprietary_codecs = true
strip_absolute_paths_from_debug_symbols = true
symbol_level = 1
target_os = "android"
use_goma = true

Then, from your src directory, you should be able to run the following command to run the tests (with a rooted Android device attached to your workstation):

tools/perf/run_benchmark memory.top_10_mobile -v --browser=android-chromium --output-format=histograms

We have seen some pretty strange/unexplained memory increases in the past, so I wouldn't get too worried about this one, particularly since the increase is so small.

+CC tiborg@ who might be able to help further, as I'll be out for ~1 month.
Cc: -tiborg@chromium.org
Owner: tiborg@chromium.org
Based on #8:
> We have seen some pretty strange/unexplained memory increases in the past, so I wouldn't get too worried about this one, particularly since the increase is so small.

I'm inclined to mark this wontfix.

tiborg@, making you owner to decide/approve a wontfix on this.
Status: WontFix (was: Assigned)
I'm fine with closing this.
Components: Internals>XR

Sign in to add a comment