Restore navigator.hardwareConcurrency

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Tadeu Zagallo]

Navigator.hardwareConcurrency was removed in r219379, but it is now used by emscripten to implement std::thread::hardware_concurrency()

<rdar://85023911>

[Alexey Proskuryakov]

r219379 doesn't explain the reasons why it was never enabled in WebKit, but there are multiple.

- Various privacy concerns (fingerprinting, socioeconomic targeting).

- Inconsistent meaning of return value ("logical processors" is very different when there is or isn't hyperthreading, or any other non-uniformity).

- Inherits the downsides of native API, which itself is harmful (no accounting for system load or other prioritization decisions that the system may be making, unlike with higher level APIs).

[Tadeu Zagallo]

Created attachment 444866 [details]
Patch

[Saam Barati]

Comment on attachment 444866 [details]
Patch

r=me

[Tadeu Zagallo]

Created attachment 445732 [details]
Patch

[Tadeu Zagallo]

Created attachment 445767 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 445767 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445767&action=review

> Source/WebCore/page/NavigatorBase.cpp:188
> +        // If machines with more than 8 cores become commonplace, we should bump this number.
> +        // see https://bugs.webkit.org/show_bug.cgi?id=132588 for the
> +        // rationale behind this decision.

That is a bug with 100 comments to go through. What is the specific rationale for not always returning 2 or 4, for example? Why do we need even one bit of fingerprinting information?

[Tadeu Zagallo]

(In reply to Alexey Proskuryakov from comment #6)
> That is a bug with 100 comments to go through. What is the specific
> rationale for not always returning 2 or 4, for example? Why do we need even
> one bit of fingerprinting information?

That comment was kept from when I reverted the patch that deleted the original implementation, maybe it could use an update, although I think it's still mostly accurate. The information can be used through the pthread API to determine the number of workers to be started, so it's a compromise between giving an answer that is at least somewhat useful while mitigating fingerprinting issues.

Regarding your other points, I read most of the discussion in the original bug (https://bugs.webkit.org/show_bug.cgi?id=132588), it looks like the same discussion was had back in 2014 and I believe the conclusion was that regardless of other (better or not) approaches to concurrency, there's still a need for this API.

[Alexey Proskuryakov]

> it's a compromise between giving an answer that is at least somewhat useful while mitigating fingerprinting issues.

This makes sense in the abstract. However, we usually make performance decisions based on measurements, and trading off some amount of privacy for an unknown amount of performance feels icky. We also have a wider mix of desktop computers with and without multithreading (because of Apple Silicon Macs), making it harder for developers to guess what to do with the number of cores reported by this API.

It is true that this is partly a re-hash of the prior discussion. That one hasn't had a clear conclusion though - the code was added as an experimental feature, but never enabled by default AFAIK.

The fact that no one seems to have looked into creating a better API since 2014 could mean that this one is good enough, or that it hasn't become impactful enough to show its limitations in other browsers.

[Tadeu Zagallo]

(In reply to Alexey Proskuryakov from comment #8)
 
> However, we usually make performance decisions based on measurements, and trading off some amount of privacy for an unknown amount of performance feels icky.

This isn't a performance decision though, it's a compatibility issue. I arrived at this API by debugging a website not working. WebAssembly was trapping because it was using the result of this API which was unexpectedly 0 (well, undefined which was eventually converted to 0).

[Alexey Proskuryakov]

This part was specifically about returning values 4 or 8, and why we cannot return the same value always. That seems like a performance vs. privacy tradeoff to me.

[Saam Barati]

(In reply to Alexey Proskuryakov from comment #10)
> This part was specifically about returning values 4 or 8, and why we cannot
> return the same value always. That seems like a performance vs. privacy
> tradeoff to me.

Yes, it is. We decided this bit of information is an ok tradeoff to make. I'll link you to the discussion we had with John and Brent.

[Alexey Proskuryakov]

Right, what I'm saying is that this decision appears to have been made in a way that appears inconsistent with how we usually make performance related decisions. To be clear, I'm not stopping you, but I'm not feeling good about it.

[Sam Weinig]

(In reply to Saam Barati from comment #11)
> (In reply to Alexey Proskuryakov from comment #10)
> > This part was specifically about returning values 4 or 8, and why we cannot
> > return the same value always. That seems like a performance vs. privacy
> > tradeoff to me.
> 
> Yes, it is. We decided this bit of information is an ok tradeoff to make.
> I'll link you to the discussion we had with John and Brent.

Can you link it here? That seems like useful information to be in the bug and ChangeLog.

[Saam Barati]

(In reply to Sam Weinig from comment #13)
> (In reply to Saam Barati from comment #11)
> > (In reply to Alexey Proskuryakov from comment #10)
> > > This part was specifically about returning values 4 or 8, and why we cannot
> > > return the same value always. That seems like a performance vs. privacy
> > > tradeoff to me.
> > 
> > Yes, it is. We decided this bit of information is an ok tradeoff to make.
> > I'll link you to the discussion we had with John and Brent.
> 
> Can you link it here? That seems like useful information to be in the bug
> and ChangeLog.

Unfortunately it was on the Apple internal slack (and probably shouldn't have been). Maybe John can comment in this bug and share his opinion?

[EWS]

Committed r286550 (244881@main): <https://commits.webkit.org/244881@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445767 [details].