RFR: 8203227: Introduce os::processor_id() for Linux and Solaris

[David Holmes]

Hi Per,

On 16/05/2018 1:07 AM, Per Liden wrote:
> This patch introduces os::processor_id() to get the id of the CPU that 
> the caller is currently executing on. Obviously, the returned id should 
> only be viewed as a strong hint, since the information might be 
> instantly out-of-date.

Or even wrong to begin with:

https://stackoverflow.com/questions/36934736/is-sched-getcpu-reliable-on-linux

caveat-emptor! :)

> We're using this in ZGC to do various CPU-local operations. We currently

So there's a requirement that the CPU value returned by processor_id() 
is compatible with some other API's that take a processor id? Does that 
need documenting in some form?

> only need this for Linux and Solaris, which is what this patch adds. 

Are the API's that take the processor id defined in os as well? 
Otherwise it seems this functionality should just be localized to the 
specific os_<os>.cpp file from which it is used (and/or the os::<OS> class).

> Support for additional platforms can be added when needed. I didn't add 
> dummy os::processor_id() to the not-yet-implemented platforms, since I 
> think it's better to get a early compile-time error instead of a 
> Unimplemented() at runtime, if you try to use them.

I'd rather not see the runtime support of our primary platforms of 
interest fragmented like this. Are there technical challenges to doing this?

> Bug: https://bugs.openjdk.java.net/browse/JDK-8203227
> Webrev: http://cr.openjdk.java.net/~pliden/8203227/webrev.0

For Solaris there is no need to introduce Solaris::getcpuid() just 
because Linux already has that. (Unless this ends up not being needed in 
the os class of course :) ).

Are you allowing for the -1 return value on Linux or assuming it will 
never happen in practice? If the former then that should be documented 
as part of the semantics of os::processor_id(). If the latter then you 
should probably add an assert or guarantee. I suspect this is a 
historical restriction no longer relevant to us - in which case we can 
probably clean out the dynamic lookup/linking!).

Thanks,
David

> /Per