New issue
Advanced search Search tips

Issue 618222 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 618679
Owner:
Closed: Jun 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

11.9% regression in speedometer at 398015:398050

Project Member Reported by hablich@chromium.org, Jun 8 2016

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=618222

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


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

chromium-rel-win7-single
Kicked off another bisect with count = 40.
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jun 24 2016

Mergedinto: 618679
Status: Duplicate (was: Assigned)

===== 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