Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature popularity stackrank chart looks wrong #545

Closed
nyaxt opened this issue Apr 2, 2018 · 26 comments
Closed

Feature popularity stackrank chart looks wrong #545

nyaxt opened this issue Apr 2, 2018 · 26 comments

Comments

@nyaxt
Copy link

nyaxt commented Apr 2, 2018

https://www.chromestatus.com/metrics/feature/popularity says that the most popular feature implemented in Chromium is CSSSelectorWebkitProgressBar with 1.36%.
However, CookieGet, which is used in >85% of the websites doesn't appear in the stackrank.

@ebidel
Copy link
Contributor

ebidel commented Apr 2, 2018

Weird. These are back for me:

screen shot 2018-04-02 at 8 41 18 am

We didn't make any changes to the site overnight :) Maybe there were backend issues with GCP and the datastore.

@ebidel
Copy link
Contributor

ebidel commented Apr 2, 2018

Please reopen if you see this again.

@ebidel ebidel closed this as completed Apr 2, 2018
@ebidel
Copy link
Contributor

ebidel commented Apr 2, 2018

From reading blink-dev threads, looks like metrics where changed from under us. Reopening until we figure out what's going on.

The % are also over 100% :(

@ebidel ebidel reopened this Apr 2, 2018
@clelland
Copy link
Collaborator

clelland commented Apr 3, 2018

cc: @loonybear -- in case this is related to recent UseCounter changes in Blink.

@loonybear
Copy link

Yea sorry about that. The % over 100% thing should have been fixed and rolled out soon (M67).

@ebidel
Copy link
Contributor

ebidel commented Apr 3, 2018

Thanks @loonybear. Guess we'll keep this open and watch for m67 to hit stable. Hopefully this fixes itself.

@phistuck
Copy link
Contributor

phistuck commented Apr 3, 2018

@ebidel and @loonybear - those percentages are feeding investigations and decisions on Blink intents, can a fix be merged, or can something else be done so that the numbers would be trustworthy?

@loonybear
Copy link

Most features are still being reported correctly.
Big feature usage shift was due to a few bugs that caused the pagevisits being off by 30%.

Over 100% usage were because features being logged before page commits and they could be a) logged multiple times per page or b) counted even though shouldn't be logged at all (features from a non http/https page or a new tab page or etc). But those features are rare, as most features are logged after page commits.

We can certainly merge it earlier but I don't think the percentages will be affecting blink intents process much.

@ebidel
Copy link
Contributor

ebidel commented Apr 3, 2018

@phistuck I'm not worried about a temporary hiccup in a few metrics. Plus, anything that high wouldn't be considered for a deprecation/removal :)

@phistuck
Copy link
Contributor

phistuck commented Apr 4, 2018

@ebidel - having a few metrics above 100% looks like the tip of the iceberg and makes me (at least) not trust the rest of the results. Other things might have been blown out of proportion as well, but simply did not make it to over 0.1% more than the usual.
This reduces the deprecation/removal opportunities, which sounds like you are against for some reason (I know they can be hard for the few developers that use those features, but they do benefit the platform overall, after all).

@loonybear
Copy link

Re "having a few metrics above 100% looks like the tip of the iceberg and makes me (at least) not trust the rest of the results":
I totally understand that people will tend to distrust the results because of that. However, the reason for the over 100% usage is due to some features measured before load commits, which are very rare cases.

The change from previous to now:
Previously: PageVisits were not counted correctly, off by 30%
Now: PageVisits is correctly counted (reduced by 30%)
There was no change made to the count of any other features, which means the previous stats for the usage (feature count / page visits count) was wrong initially.
Now feature usage might be inflated by 30%, but for most cases, feature usage is more accurate despite some of the usage is over 100%.

Re "reduces the deprecation/removal opportunities, which sounds like you are against for some reason", I don't think Eric was against deprecation/removal. It is just normally when a feature has a usage >=0.1%, we consider the impact to be too large to change. In other word, it will break the web. For those features having usage over 100%, even with the fix, I doubt the usage will be reduced to below 0.1%, which means we are not going to change the behaviour of those features anyways. For other features, previously we were using the wrong stats (current usage / 1.3). And now we are using the right stats. But the 1.3 difference will not shift a feature usage from 0.1% to 0.01% or from 1% to 0.1% so it doesn't really affect the intent process much.

I will look into if I can merge the change to an earlier version. But hopefully my comments above can reduce your concern.

Thanks

@ebidel
Copy link
Contributor

ebidel commented Apr 4, 2018

Thanks for the explanation @loonybear. That's what I was referring to :)

From the sound if it, the data is getting more accurate. In the long-term, that puts us in a better place than before. Short-term, we have a few features reporting higher-than-expected numbers. It's worth noting the opposite has been true in the past. We under reported for a long time e.g. TheJump from last year:

screen shot 2018-04-04 at 10 32 07 am

Hopefully issues like this don't happen as often in the future, but we'll always be fine tuning the data.

@TakayoshiKochi
Copy link

Getting back here since M67 has been shipped for a few days.
I don't think M67 rolled out to everyone yet, but still, considering the stats are calculated ~24hours there should be some change ...

Screenshot of the stack rank today.

wuvdw0vetzg

@ebidel
Copy link
Contributor

ebidel commented Jun 4, 2018

The numbers that we're pulling from UMA still are inflated. You can see that the entry on 2018-06-01 is still 142%.

screen shot 2018-06-04 at 11 29 28 am

@loonybear Are these numbers starting to stabilizing on internal UMA?

@loonybear
Copy link

loonybear commented Jun 6, 2018 via email

@TakayoshiKochi
Copy link

Any updates? Today the top-ranking one (SecureContextCheckFailed) is 177% 😦

@ebidel
Copy link
Contributor

ebidel commented Jun 19, 2018

UMA is coming down internally not that the new stable is taking over.

screen shot 2018-06-19 at 8 14 17 am

There's a couple of days of delay on our side. We should hopefully start to see better numbers by end of week.

@TakayoshiKochi
Copy link

TakayoshiKochi commented Jun 29, 2018

Today the top "CleanScriptElementWithNonce" is ~97% (< 100%), so it's fixed now...?

The old top ranked "SecureContextCheckFailed" is ~25% now, down from 177% 10 days ago,
which is a sharp drop.

image

Shall we ignore the date between Mar. 20 ~ Jun. 15?
And it would be nice we have another note like one for 2017-10-26.

@ebidel
Copy link
Contributor

ebidel commented Jun 30, 2018

Oh great! Looks like it's corrected itself. I'll leave this bug open as a reminder to annotate the graph.

@loonybear
Copy link

loonybear commented Jul 9, 2018 via email

@progers
Copy link
Contributor

progers commented Aug 14, 2018

Emil (eae) and I were looking at graphs today and they still seem to be inflated. Should this have been fixed or did this bug surface again?

@loonybear
Copy link

loonybear commented Aug 14, 2018 via email

@progers
Copy link
Contributor

progers commented Aug 14, 2018

A lot of them are over 100%:
https://www.chromestatus.com/metrics/css/popularity

For example, display is reported as being on ~150% of pages:
https://www.chromestatus.com/metrics/css/timeline/popularity/4

@loonybear
Copy link

loonybear commented Aug 14, 2018 via email

@progers
Copy link
Contributor

progers commented Aug 14, 2018

Thanks for the quick response. If I understand correctly, this will be fixed when M69 is promoted to stable (near the end of this month)?

@loonybear
Copy link

loonybear commented Aug 14, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants