Issue metadata
Sign in to add a comment
|
11.9% regression in speedometer at 398015:398050 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 24 2016
Kicked off another bisect with count = 40.
,
Jun 24 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Making cookies eviction quotas match spec Author : maksim.sisov Commit description: According to the section 3 and section 5 in https://tools.ietf.org/html/draft-west-cookie-priority- 00#section-3, the old cookie monster implementation doesn't maintain priority quotas correctly. This CL makes sure there will always be at least 30 low priority cookies, 50 mid priority and 70 high priority cookies if there had been enough of them before the eviction. Please note, secure cookies are more important than non-secure per https://tools.ietf.org/html/draft-west-leave-secure-cookies-alone. A small example: if there are 70 cookies: 37 non-secure low priority and 33 secure low priority cookies, 37 non-secure cookies are deleted during the first round and other 3 secure low priority cookies are deleted during the following round preserving 30 low priority cookies according to the quota of those specific cookies. For a bigger example that fully complies with the implementation, check the unittests. Before the fix, the unittests were just adjusted to the behavior of cookies eviction. For example, if we take the following unittest: Round 1 => 10L; round 2 => 11M, 10L; round 3 => none. TestPriorityCookieCase(cm.get(), "11HN 10MN 20LN 110MN 20LN 10HN", 20U,109U, 21U, 150U, 0U); The problem here was that there were only 40 low priority cookies, but the quota was not preserved for those cookies. First, 10 low priority cookies were deleted and then more 10 low priority cookies were deleted leaving only 20 of them, which was less than the quota (30 low priority cookies). It happened because the eviction algorithm didn't know how many cookies of a specific priority were deleted and it had always started to delete all the cookies from the beginning of the container removing even those cookies that shouldn't have been deleted. After we land this CL, we can have cookies in any order and high priority cookies will be eventually deleted in order to avoid excess of high priority cookies by some applications within the same domain. Thus, after the eviction algorithm runs, we should have at least 30 low, 50 mid and 70 high priority cookies if we had sufficient amount of them in the beginning. BUG= 609550 Review-Url: https://codereview.chromium.org/1976073002 Cr-Commit-Position: refs/heads/master@{#398049} Commit : fe1fea6df040fff38a3a4567e20a3d5847e83170 Date : Mon Jun 06 16:53:15 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@398014 230.253 5.80385 5 good chromium@398032 227.851 3.80008 5 good chromium@398041 225.461 3.68219 5 good chromium@398046 225.704 3.53458 5 good chromium@398048 228.653 4.64324 5 good chromium@398049 245.42 5.05859 5 bad <-- chromium@398050 245.168 5.03065 8 bad Bisect job ran on: win_perf_bisect Bug ID: 618222 Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests speedometer Test Metric: VanillaJS-TodoMVC/VanillaJS-TodoMVC Relative Change: 5.14% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect/builds/6614 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9008939689476615840 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5822943859310592 | 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! |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by hablich@chromium.org
, Jun 8 2016