RFR: 8295661: CompileTask::compile_id() should be passed as int

[Damon Fenacci]

On Mon, 12 Dec 2022 18:50:40 GMT, Tom Rodriguez <never at openjdk.org> wrote:

>> Changed return type of `CompileTask::compile_id()` from `int` to `uint`.
>> Also modified the type everywhere `compile_id` is used (parameters, local variables, string formatting, constant assignments) *as much as possible*.
>> Added *asserts* to check for valid value range where not possible.
>
> Why isn't the proper fix to simply change the `CompileTask::_compile_id` to an int?  Every other piece of code thinks it's an int so we should that declaration be controlling?  `CompileBroker::assign_compile_id` returns int which seems much more definitive to me.  Is there some known problem with running out of compile ids in the in signed range?
> 
> If this range is really being promoted to unsigned int then I think this needs to be properly exposed all the way through the JVMCI API with some new API to expose it as a long.  Simply adding asserts for something we believe can legally occur really isn't sufficient.

@tkrodriguez thanks a lot for your insight. I see your point: the pragmatic approach you propose is surely more appropriate. Reverting the change.

@tkrodriguez @TobiHartmann @dougxc I've reverted the previous changes and changed `CompileTask::_compile_id` to an `int` instead. I've also changed the type of the compile id from `uint` to `int` in a few other places to make things consistent.

> If we were really worried about overflow then I think it should be promoted to a jlong since the difference between the signed and unsigned range is really relatively small.  I think the required changes to JVMCI could be made in a backward compatible way where there is new API treats it as long.  The primary place where this is exposed is in `HotSpotCompilationRequest.getId`.  The old API could be made to throw an exception if it encounters a too large id.  Anyway if we want to address that issue I can probably provide the required JVMCI changes.

Thanks @tkrodriguez @TobiHartmann @dougxc for the reviews!

-------------

PR: https://git.openjdk.org/jdk/pull/11630