New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 614367 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

34.5% regression in service_worker.service_worker_micro_benchmark at 395212:395223

Project Member Reported by falken@chromium.org, May 24 2016

Issue description

See the link to graphs below.
 

Comment 1 by falken@chromium.org, May 24 2016

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgzJuAoQkM


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

chromium-rel-win7-dual

Comment 2 by falken@chromium.org, May 24 2016

Components: Blink>ServiceWorker

Comment 3 by falken@chromium.org, May 26 2016

https://chromeperf.appspot.com/buildbucket_job_status/9011776611467111904

  Status of the bisect job 9011776611467111904: COMPLETED, FAILURE

Comment 4 by falken@chromium.org, May 26 2016

Trying again:
	
2016-05-26 10:32:41 Bisect started; track progress at https://chromeperf.appspot.com/buildbucket_job_status/9011607120888379632
2016-05-26 10:32:30 Bisect started; track progress at https://chromeperf.appspot.com/buildbucket_job_status/9011607133377581696

Comment 6 by falken@chromium.org, May 26 2016

Cc: yhirano@chromium.org
yhirano: https://codereview.chromium.org/1993033003 just flips a flag, right? It shouldn't affect our microbenchmark that does respondWith(new Response()).

Comment 7 by falken@chromium.org, May 26 2016

Labels: Performance-Sheriff-Regressions
Performance sheriff: Can you tell me if the perf bots run Chrome with Blink "stable" or "experimental" runtime flags on? I want to know whether this kind of change can affect the bot: https://codereview.chromium.org/1993033003/diff/100001/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Comment 8 by falken@chromium.org, May 26 2016

Cc: nedngu...@google.com
+nednguyen directly (not sure if I got the right label for perf sheriff)
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, May 26 2016


===== BISECT JOB RESULTS =====
Status: completed


=== Bisection aborted ===
The bisect was aborted because The metric values for the initial "good" and "bad" revisions do not represent a clear regression.
Please contact the the team (see below) if you believe this is in error.

=== Warnings ===
The following warnings were raised by the bisect job:

 * Bisect failed to reproduce the regression with enough confidence.

===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N   Good?
chromium@395211  2.34028  0.612287  18  good
chromium@395223  2.68676  0.753882  17  bad

Bisect job ran on: win_perf_bisect
Bug ID: 614367

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests service_worker.service_worker_micro_benchmark
Test Metric: concurrent_1_response_99_percentile/concurrent_1_response_99_percentile
Relative Change: 19.02%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect/builds/6536
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9011607120888379632


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=4928580828528640

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, May 26 2016

Mergedinto: 614472
Status: Duplicate (was: Assigned)

===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Ship "Response construction with ReadableStream".
Author  : yhirano
Commit description:
  
Intent-To-Ship thread: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/0S-jPuCy8ws
BUG= 564479 ,  596832 

Review-Url: https://codereview.chromium.org/1993033003
Cr-Commit-Position: refs/heads/master@{#395223}
Commit  : e0cb6a355deab56251cea1ac5b2bcc4b605c76e2
Date    : Sat May 21 01:36:08 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev    N  Good?
chromium@395205  1.858    0.0776692  5  good
chromium@395214  1.848    0.159437   5  good
chromium@395219  1.917    0.143248   5  good
chromium@395221  1.95938  0.199793   8  good
chromium@395222  2.01312  0.179343   8  good
chromium@395223  2.433    0.230884   5  bad    <--
chromium@395240  2.47     0.255881   5  bad

Bisect job ran on: mac_10_11_perf_bisect
Bug ID: 614367

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests service_worker.service_worker_micro_benchmark
Test Metric: concurrent_1_response_99_percentile/concurrent_1_response_99_percentile
Relative Change: 32.94%
Score: 99.0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_11_perf_bisect/builds/646
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9011607133377581696


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5812245787836416

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!
Status: Assigned (was: Duplicate)
I'd rather not merge to that one.
To question in #7, for windows, Telemetry uses finch trial flags in Telemetry uses finch trial flags in https://code.google.com/p/chromium/codesearch#chromium/src/testing/variations/fieldtrial_testing_config_win.json

You can find the flag telemetry directly uses to run chrome from the log:
(INFO) 2016-05-26 03:38:55,730 desktop_browser_backend.Start:274  Starting Chrome ['C:\\b\\build\\slave\\Win_7_x64_Perf__4_\\build\\src\\out\\Release_x64\\chrome.exe', u'--force-fieldtrial-params=DisallowFetchForDocWrittenScriptsInMainFrame.DocumentWriteScriptBlockGroup:disallowFetchForDocWrittenScriptsInMainFrameOnSlowConnections/true,AutomaticTabDiscarding.Enabled_Once_10-gen2:MinimumProtectionTime/600,VarationsServiceControl.Interval_30min:fetch_period_min/30,PreRead.NoPrefetchArgument2:NoPrefetchArgument/true,QUIC.Enabled:delay_tcp_race/true,NetworkQualityEstimator.Enabled:Ethernet%2EDefaultMedianRTTMsec/106/4G%2EDefaultMedianKbps/1756/4G%2EDefaultMedianRTTMsec/159/None%2EDefaultMedianRTTMsec/272/2G%2EDefaultMedianRTTMsec/1337/3G%2EDefaultMedianKbps/658/3G%2EDefaultMedianRTTMsec/297/Bluetooth%2EDefaultMedianRTTMsec/135/Unknown%2EDefaultMedianKbps/1916/2G%2EDefaultMedianKbps/81/Unknown%2EDefaultMedianRTTMsec/121/None%2EDefaultMedianKbps/567/Bluetooth%2EDefaultMedianKbps/449/WiFi%2EDefaultMedianRTTMsec/168/Ethernet%2EDefaultMedianKbps/3263/WiFi%2EDefaultMedianKbps/2736/HalfLifeSeconds/60,ReportCertificateErrors.ShowAndPossiblySend:sendingThreshold/1%2E0,SafeBrowsingIncidentReportingService.Enabled:collect_reg_data/true,SafeBrowsingUpdateFrequency.UpdateTime15m:NextUpdateIntervalInMinutes/15,AutofillProfileOrderByFrecency.EnabledLimitTo3:limit/3', u'--force-fieldtrials=DisallowFetchForDocWrittenScriptsInMainFrame/DocumentWriteScriptBlockGroup/ChildAccountDetection/Disabled/SdchPersistence/Enabled/PasswordSeparatedSigninFlow/Enabled/InstanceID/Enabled/EnableSessionCrashedBubbleUI/Enabled/MojoChannel/Enabled/AutomaticTabDiscarding/Enabled_Once_10-gen2/V8CacheStrategiesForCacheStorage/none/AutofillFieldMetadata/Enabled/DefaultBrowserInfobar/SettingsText/StrictSecureCookies/Enabled/SyncHttpContentCompression/Enabled/GoogleNow/Enable/SafeBrowsingReportPhishingErrorLink/Enabled/ChromotingQUIC/Enabled/MainFrameBeforeActivation/Enabled/SSLCommonNameMismatchHandling/Enabled/EnableMediaRouter/Enabled/VarationsServiceControl/Interval_30min/PageRevisitInstrumentation/Enabled/PasswordBranding/SmartLockBrandingSavePromptOnly/SafeBrowsingIncidentReportingServiceFeatures/WithSuspiciousModuleReporting/EnableGoogleCachedCopyTextExperiment/Button/RefreshTokenDeviceId/Enabled/WebRTC-H264WithOpenH264FFmpeg/Enabled/MaterialDesignDownloads/Enabled/PreRead/NoPrefetchArgument2/TabSyncByRecency/Enabled/PasswordManagerSettingsMigration/Enable/ResourcePriorities/Launch50pct_11011_1_1_10/WebFontsIntervention/Enabled/use-new-media-cache/Enabled/ChromeDashboard/Enabled/WebRTC-LocalIPPermissionCheck/Enabled/LocalNTPSuggestionsService/Enabled/QUIC/Enabled/NetworkQualityEstimator/Enabled/SettingsEnforcement/enforce_always_with_extensions_and_dse/CaptivePortalInterstitial/Enabled/IntelligentSessionRestore/Enabled/OfferUploadCreditCards/Enabled/TriggeredResetFieldTrial/On/NetworkTimeQueries/NetworkTimeQueriesEnabled/SchedulerExpensiveTaskBlocking/Enabled/SafeBrowsingV4LocalDatabaseManagerEnabled/Enabled/ReportCertificateErrors/ShowAndPossiblySend/SafeBrowsingIncidentReportingService/Enabled/NewProfileManagement/Enabled/ExtensionContentVerification/Enforce/OutOfProcessPac/Enabled/WebRTC-EnableWebRtcEcdsa/Enabled/IconNTP/Default/ExtensionInstallVerification/Enforce/AutofillClassifier/Enabled/WebRTC-PeerConnectionDTLS1.2/Enabled/BrowserBlacklist/Enabled/EnableMediaRouterWithCastExtension/Enabled/PasswordGeneration/Disabled/SRTPromptFieldTrial/On/GoogleBrandedContextMenu/branded/PreconnectMore/Enabled/ExtensionActionRedesign/Enabled/SafeBrowsingUpdateFrequency/UpdateTime15m/TokenBinding/TokenBinding/AutofillProfileOrderByFrecency/EnabledLimitTo3', u'--disable-features=DocumentWriteEvaluator', u'--enable-features=UsePasswordSeparatedSigninFlow,AutomaticTabDiscarding,MainFrameBeforeActivation,IncidentReportingModuleLoadAnalysis,IncidentReportingSuspiciousModuleReporting,WebRTC-H264WithOpenH264FFmpeg,use-new-media-cache,NetworkTimeServiceQuerying,IncidentReportingDisableUpload,WebRTC-EnableWebRtcEcdsa,PreconnectMore,token-binding', '--enable-net-benchmarking', '--metrics-recording-only', '--no-default-browser-check', '--no-first-run', '--enable-gpu-benchmarking', '--disable-background-networking', '--ignore-certificate-errors', '--host-resolver-rules=MAP * 127.0.0.1,EXCLUDE localhost', '--testing-fixed-http-port=55204', '--testing-fixed-https-port=55205', '--no-proxy-server', '--disable-component-extensions-with-background-pages', '--disable-default-apps', '--remote-debugging-port=0', '--enable-crash-reporter-for-testing', '--window-size=1280,1024', u'--ppapi-flash-path=C:\\b\\build\\slave\\Win_7_x64_Perf__4_\\build\\src\\third_party\\adobe\\flash\\binaries\\ppapi\\win_x64\\pepflashplayer.dll', '--user-data-dir=c:\\users\\chrome~1\\appdata\\local\\temp\\tmphrqdx4', 'about:blank']


(This is from: https://uberchromegw.corp.google.com/i/chromium.perf/builders/Win%207%20x64%20Perf%20(4)/builds/3890/steps/service_worker.service_worker_micro_benchmark/logs/stdio)

Sadly the log when this regression happens is no longer stored so I can't access it. 
Thanks Ned. Since --enable-experimental-web-platform-features is not on, probably this change made a difference (in Blink I'm accustomed to the "experimental" features being enabled for continuous testing like layout tests https://www.chromium.org/blink/runtime-enabled-features, but it probably makes sense that perf bots don't do the same).
Cc: domenic@chromium.org
cc: domenic

According to falken@ this test runs https://github.com/horo-t/Service-Worker-Performance/blob/0cc35c2398526665399ca99fe53147ff81101408/request-latency-worker.js .

(Personally I think that new Response({status: ...}) is wrong and should be fixed to new Response(null, {status: ...}))
As we are starting using v8-extra readable stream, it is possible that creating a Response with non-null body takes more time than ever.
Yep, I filed  issue 614985  for that.

Regardless, I wouldn't worry much about this regression, unless you see some obvious slow part. Any slight change in the constructor path is probably going to create a change in this microbenchmark.

We should probably just fix the order of params and make that the new baseline.
Cc: shimazu@chromium.org
Domenic: does using v8-extra have a performance cost?

The SW microbenchmark took a 10-30% hit when Response constructor started using ReadableStream. The microbenchmark basically performs concurrent XHR requests that the SW responds to using new Response('hello').

The current SW does new Response({status:}) but shimazu@ found that changing the object to a simple string still would show the regression.
Cc: littledan@chromium.org
+littledan@ as someone who I know is involved in improving promise performance.

V8 extras should in general not have a performance cost; usually they give a benefit in situations like this. Looking into this one guess as to what's happening here is that more work is being done up-front instead of lazily, since the JS stream object is created immediately on Response construction instead of lazily as a wrapper around the C++ ReadableStream.

I tend to agree with

> Regardless, I wouldn't worry much about this regression, unless you see some obvious slow part. Any slight change in the constructor path is probably going to create a change in this microbenchmark.

I did some profiling on the benchmark and got the attached results. 25% of the benchmark is attributable to the Response constructor; 8.61% is binding.createReadableStreamWithExternalController (called by Response, so part of that 25%).

We could consider fast-paths for start returning `undefined` and just post a microtask instead of converting it to a promise. But I guess largely we're working around promises being slow by doing that.

So in conclusion:

- There is some understandable regression in this microbenchmark, probably because more work is being done up-front, including promise processing which is slow because V8's promise implementation is slow
- Promise overhead accounts for over 50% of the benchmark; ReadableStream is responsible for ~8%, of which large portions are also promise-related.
- We could consider trying to special-case and bypass the promise machinery as an optimization, but absent compelling non-microbenchmarks, it might be best to wait until promises can be further optimized, as that will have a much larger impact across the entire benchmark and not just the 8%.
CPU-20160531T154103.cpuprofile
669 KB Download
Status: WontFix (was: Assigned)
Domenic: Thanks for your analysis. I think it makes sense to accept this microbenchmark regression since ReadableStream support is worth it, and look forward to a faster promise implementation.
Labels: -Performance-Sheriff-Regressions Performance-Sheriff

Sign in to add a comment