Issue metadata
Sign in to add a comment
|
34.5% regression in service_worker.service_worker_micro_benchmark at 395212:395223 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
May 24 2016
,
May 26 2016
https://chromeperf.appspot.com/buildbucket_job_status/9011776611467111904 Status of the bisect job 9011776611467111904: COMPLETED, FAILURE
,
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
,
May 26 2016
Blame range: https://chromium.googlesource.com/chromium/src/+log/e81c454dee385210aba6f8685c5c3b0a87e0e970%5E..e0cb6a355deab56251cea1ac5b2bcc4b605c76e2?pretty=fuller Nothing looks suspicious.
,
May 26 2016
yhirano: https://codereview.chromium.org/1993033003 just flips a flag, right? It shouldn't affect our microbenchmark that does respondWith(new Response()).
,
May 26 2016
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
,
May 26 2016
+nednguyen directly (not sure if I got the right label for perf sheriff)
,
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!
,
May 26 2016
===== 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!
,
May 26 2016
I'd rather not merge to that one.
,
May 26 2016
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.
,
May 26 2016
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).
,
May 26 2016
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.
,
May 26 2016
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.
,
May 31 2016
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.
,
May 31 2016
+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%.
,
Jun 3 2016
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.
,
Feb 3 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by falken@chromium.org
, May 24 2016