Rename bytes to userAgentSpecificBytes

ulan commented 4 years ago

Opening this issue to discuss the naming of the fields. As mentioned in Alternatives we could add a userAgentSpecific prefix to bytes and (?) attribution. We have three options.

Option 1 (the current):

{
  bytes: 40*MB,
  breakdown: [
    {
      bytes: 40 * MB,
      attribution: ['https://foo.com'],
      userAgentSpecificTypes: ['Window', 'JS']
    }
  ]
}

Option 2 (change bytes, but keep attribution):

{
  userAgentSpecificBytes: 40*MB,
  breakdown: [
    {
      userAgentSpecificBytes: 40 * MB,
      attribution: ['https://foo.com'],
      userAgentSpecificTypes: ['Window', 'JS']
    }
  ]
}

Option 3 (change bytes and attribution):

{
  userAgentSpecificBytes: 40*MB,
  breakdown: [
    {
      userAgentSpecificBytes: 40 * MB,
      userAgentSpecificAttribution: ['https://foo.com'],
      userAgentSpecificTypes: ['Window', 'JS']
    }
  ]
}

The options are listed in the order of my preference. My reasoning is that web compat risks with bytes are small because the value will likely change even between two calls on the same browser for a non-trivial web page. And we can communicate the message about byte being not comparable in documentation.

What options do folks prefer?

Based on the comment posted by @smaug---- in https://github.com/mozilla/standards-positions/issues/281#issuecomment-603192617

domenic commented 4 years ago

Option 4: change the method name, and remove the prefix from all the fields.

(I prefer option 1 for the reasons you listed, but not strongly.)

smaug---- commented 3 years ago

I wouldn't expect documentation being too effective here. I kind of like option 4.

ulan commented 3 years ago

With option 4 the name would be something like: performance.measureUserAgentSpecificMemory() or performance.measureMemoryUserAgentSpecific().

I would be okay with such renaming to resolve Mozilla's concern. @domenic wdyt? Any preference for the naming variant?

domenic commented 3 years ago

No serious concerns, although I'd color the bikeshed "measureMemoryUASpecific()". I think we have plenty of precedence for abbreviating "user agent" in web platform APIs.

I overall think this is unnecessary, as pretty much all APIs under performance are UA-specific, and we haven't changed their names in a similar way. (In fact I'm a bit worried that this is going to set a precedent, where all performance APIs created after 2020 are going to have "UASpecific" in their name, and all ones before are not?) So I'd encourage getting a temperature check from the web perf working group, including Mozilla representatives there, so that we know how this sort of change impacts their plans for future performance APIs.

smaug---- commented 3 years ago

This is quite different to most of the performance APIs. They are usually measuring something timing related, which have lots of fluctuation already because of network condition or whatever is happening in the OS. Measuring memory usage is all about UA.

ulan commented 3 years ago

We will discuss the naming at the next Web Perf WG session: Jan 7, 10 am PST:

ulan commented 3 years ago

I'll try to summarize Web Perf WG discussion here. (video recording).

There two issues that we want to mitigate by adding "UA-specific":

Web compatibility risk: a web page may attempt to hardcode a memory threshold and change its runtime behavior depending on the result of the API call (e.g. switch to low-memory mode). Since the web page does not break, but may become less optimal, this risk seems lower compared to userAgentSpecificTypes. The intended use of the API is to collect telemetry data for regression detection without affecting the runtime behavior.
Result interpretation risk: users may attempt to compare the results from different browsers.

We can group the potential users of the API as follows:

Users that do not understand what "UA-specific" means and do not know that "bytes" are UA-specific.
Users that understand "UA-specific", but do not know that "bytes" are UA-specific.
Users that understand "UA-specific" and know that "bytes" are UA-specific.

There were concerns that group 1 may be larger than group 2. My personal opinion is that most of the users will be in group 3 because the API is gated behind cross-origin isolation which requires non-trivial effort to set up in a website.

I presented three naming options:

Option A (keep the existing naming): performance.measureMemory and result.bytes.
Option B (UA-specific in the function name): performance.measureUASpecificMemory and result.bytes.
Option C( UA-specific in the result fields): performance.measureMemory and result.uaSpecificBytes.

Option C was ruled out right away, so the discussion focused on A vs B. Most people except one favored option A.

yoavweiss commented 3 years ago

@smaug---- @bdekoz - thoughts on the above?

smaug---- commented 3 years ago

Web compat risk sounds quite high to me. If web compat issues lead to worse behavior, that is pretty bad for the users. Intended use of the API doesn't really matter, web sites will use the API whatever way they want to use it.

What isn't explained here is why performance.measureMemory would be any better in any way than something like performance.measureUASpecificMemory which clearly expresses better what is about to happen.

yoavweiss commented 3 years ago

Web compat risk sounds quite high to me. If web compat issues lead to worse behavior, that is pretty bad for the users.

I agree on that in general, but it might be good to outline what the web compat risk here actually is and how we expect the variable name to modify it.

The theory above is that developers may use the memory consumption of their renderer as an indication on the memory levels of the system. That seems unlikely to me for the following reasons:

Device Memory already provides developers with means to have a rough estimate of the system's memory.
Knowing how much memory the current page takes doesn't provide much useful info on the system's available memory
if developers e.g reduce their memory consumption if the reported values are above X, would that really create web compat issues? Do we expect order-of-magnitude difference in reporting between different UAs?

yoavweiss commented 3 years ago

@smaug---- friendly ping on the above!

smaug---- commented 3 years ago

I could easily see some web site adding some hard limit when after that limit they start to do some aggressive release on img objects or what not and that could significantly decrease user experience, say when scrolling the page.

smaug---- commented 3 years ago

And especially for things like img data, or style data or what not, I can see different options for implementations - one could share the image data across processes or one might not do that. That could easily lead to order-of-magnitude differences (depending on how the shared data is reported).

ulan commented 3 years ago

I think the scenario is unlikely because web pages do not have any incentives to eagerly release memory at the cost of latency. (Because any released memory will likely be taken by other tabs -- kind of a tragedy of the commons)

Since the API is gated behind cross-origin isolation, I expect only experienced developers who care about memory leaks and regression to use it.

That said, we cannot exclude the mentioned scenario. If it is a blocker for Mozilla, I would be okay with UASpecific.

@achristensen07, @domenic, @mmocny, @npm1, @yoavweiss: Would you be okay with renaming the API to performance.measureUASpecificMemory (or performance.measureUserAgentSpecificMemory) ?

domenic commented 3 years ago

I think it would be very surprising if developers did the kind of thing @smaug---- described, and I think it would be even more surprising if they decided not to do them because the name of the API included "UASpecific". I would prefer if we found a single example of a web developer that thought memory measurement or other performance APIs were always going to give the same results across UAs, before making such a change.

But ultimately, I don't want to block forward progress. If this is how the Web Perf Working Group wants to annotate their performance APIs that deliver UA-specific results going forward, then I'm not going to stand in their way. I hope they apply the policy consistently, though, instead of this being a lone-objector-driven wart on the web API surface where all other UA-specific performance result APIs get away without the name change.

npm1 commented 3 years ago

I still think that using UASpecific is a bit unprecedented and unnecessary. Performance values are all already pretty dependent on the user agent that computes them for a variety of reasons, so I don't buy the argument that this API is in some way special enough to warrant the annotation. I also don't think the WG has any plans to annotate other APIs with this. That said, if this will help encourage Mozilla to implement the API then I think it's a good tradeoff.

smaug---- commented 3 years ago

Another issue is that the current API has non-UA specific naming for the top level measurements, yet it uses userAgentSpecificTypes to annotate some parts. That is rather confusing. I would read that so that the top level measurement isn't UA specific, but possibly some parts of the attribution is. Given that the whole API is very much user agent specific (or rather it is specific to a certain version of a UA), it would be quite a bit clearer to hint about that strongly is method used to access the data.

It is still a bit unclear to me how web sites will use this API when I would expect numbers to change possibly quite a bit from version X to version Y of a same UA.

annevk commented 3 years ago

Looking at https://wicg.github.io/performance-measure-memory/#examples I tend to agree with @smaug----. What other API do we have that would give such disparate results? (Note that I would not count an API that returns a number that depends on how performant the implementation is, as per https://infra.spec.whatwg.org/#algorithm-conformance that is out-of-scope of standards.)

foolip commented 3 years ago

I share @domenic's concern in https://github.com/WICG/performance-measure-memory/issues/11#issuecomment-759087009 about a lone wart, and think it will be very hard to come up with a principle for what is UA-specific enough to warrant a token like "UASpecific" at the entry point. Historically APIs for exposing UA-specific behavior like video.canPlayType() don't include extra bits like that. Granted, enabling feature detection is different from exposed memory usage, but I'd still guess we regret making the API name deliberately scarier long term.

I do agree with the point however that having just userAgentSpecificTypes seems odd, that's not the only UA-specific part of this API. It would be nice to resolve that somehow, but to my eye renaming that to just type would also do the trick.

smaug---- commented 3 years ago

Based on my experience making the API name deliberately scarier is quite effective way to force the user to think about how the API works - and that is good in the cases when the API is somehow behaving in an unusual way, like in this case. I'm not aware of any other API in the web platform which may give so totally different values in different implementations (where the values depends heavily on the implementation details. video.canPlayType() is quite different case.).

ulan commented 3 years ago

Thanks all for the input! I see good points on both sides.

Since there is a strong opinion in favor of UASpecific and no strong opposition to it, let's go ahead with the renaming.

To address concerns raised in the WG discussion that "UA" may be confusing for some developers, I suggest spelling it out fully: performance.measureUserAgentSpecificMemory().

The existing userAgentSpecificTypes will be renamed to types as the new name of the function implies that all fields are UA-specific.

WICG / performance-measure-memory

Rename bytes to userAgentSpecificBytes #11