Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8256307

cleanup AvgMonitorsPerThreadEstimate and _in_use_list_ceiling types

    XMLWordPrintable

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P4
    • Resolution: Duplicate
    • Affects Version/s: 16
    • Fix Version/s: None
    • Component/s: hotspot
    • Subcomponent:
    • CPU:
      generic
    • OS:
      generic

      Description

      [~dholmes] raised this issue during the code review for JDK-8253064:

       src/hotspot/share/runtime/synchronizer.cpp
      // ObjectMonitors than the estimated average.
      //
      // Start the ceiling with the estimate for one thread:
      jint _in_use_list_ceiling = AvgMonitorsPerThreadEstimate;

      @dholmes-ora
      Why is this a jint when you use size_t for its accessor and all the other sizes that you compare with the ceiling are also size_t?
      I'm not sure size_t is right to use in these cases (do we really expect different maximums on 32-bit versus 64-bit?) but it should be all or none IMO.

      @coleenp
      Our int types are really confused. AvgMonitorsPerThreadEstimate is defined as an intx which is intptr_t and the range of it is 0..max_jint which is 0 .. 0x7fffffff . jint is long on windows (the problematic type) and int on unix. Since this is a new declaration, it probably should be something other than jint but what?
      At any rate, it should be declared as 'static'.

      @dcubed-ojdk
      _in_use_list_ceiling is a jint because we've specified the range
      as 0..max_jint and I wanted some sanity to that variable's type.

      If I change _in_use_list_ceiling to size_t, then I get a compile time
      error probably because AvgMonitorsPerThreadEstimate is an intx
      which (I think) is my only choice for a command line option.

      @fisk will have to chime in with the background on why he picked size_t.

      @dcubed-ojdk
      @coleenp - Nice catch on the missing 'static'.

      @fisk
      I typically use size_t for entities that can scale with the size of the machine's memory, so I don't have to think about whether there are enough bits. Could AvgMonitorsPerThreadEstimate be uintx instead of intx? And then maybe we don't need to declare a range, as the natural range of the uintx seems perfectly valid.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              dcubed Daniel Daugherty
              Reporter:
              dcubed Daniel Daugherty
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: