RFC 8236988 - Modular Design for JVM Flags

Ioi Lam ioi.lam at oracle.com
Wed Apr 15 06:04:46 UTC 2020


Hi Xin,

Thanks for your comments. I have uploaded a new prototype that no longer 
requires the DEFN_XXX_FLAG macros, so we don't need any of the new 
xxx_globals.cpp files anymore. You will still need one new file 
(allGlobals.cpp) file to instantiate all the flags.

http://cr.openjdk.java.net/~iklam/design/jvm_flags_overhaul/design-09a/

     make run
     make rung

 > There was a high-order macro RUNTIME_FLAGS in previous globle.hpp.
 > That's why it avoids to iterate options again & again. C-preprocessor
 > did the "codegen" job by expanding macros.
 >
 > Why do you ditch that design? it still can archive all your design goals.

As I explained in the design wiki page, my foremost goal is to 
modularize the flags.

https://wiki.openjdk.java.net/display/HotSpot/HotSpot+Command-Line+Flags+Overhaul+-+Design+Doc

I want to avoid using a single giant macro that has all the flags. Such 
a macro makes it impossible to include just the flags that a particular 
.cpp file needs. Today, every .cpp or .inline.hpp files that needs just 
a single flag must include the declaration of all 1000+ flags.

Another advantage is now flags can be easily excluded like

     #if INCLUDE_CDS
     PRODUCT_FLAG(bool, UseSharedSpaces, ...);
     #endif

This was possible before, but was extremely clumsy. So we ended up no 
doing any fine-grained subdivision and globals.hpp became the kitchen 
sink of 600+ flags.

The downside is, as you observed, it's no longer possible to put all the 
flags in a simple C++ array. I have to build a linked list during VM 
bootstrap. But that has just the cost of 1 instruction per flag. The C++ 
compiler/linker is able to fold the linked-list building to simple 
assignments:

      A->next = &B;
      B->next = &C;
      C->next = &D;
      D->next = &E;
      E->next = head;
      head = A;

And the computation of num_flags is folded into a single assignment

      num_flags += 5;

You can see this from the disassembly of the "jvm" executable from the 
new prototype. I tested with both gcc versions 5.4, 7.3 and 8.3.

So I believe the cost is negligible.

Another advantage of not using a simple C++ array is, now the FLAG_XXX 
instances can have variable size. E.g., I have

     template <typename T> class ModifiableFlag ...{
        T _min;
        T _max;
      }

so this will use less space for the smaller-sized flags such as bool and 
int.

If you want to use a C++ array of variable-sized elements, you will end 
up doing this:

     JVMFlag* flags[] = {
         & FLAG_A,
         & FLAG_B,
         & FLAG_C, ...
     };

but this will compiled to the same amount of instructions as the linked 
list ....

BTW, my plan is, after the initial integration, to build a hash table of 
the flags by statically calculating the hashcodes during build time. 
This should generate the exact same number of instructions as the 
current design (except there are 129 different heads instead of a single 
head). So the performance should be comparable/faster than an 
alphabetically sorted table. This can be done with fancy macros, or 
better, with C++ constexpr that will soon be available for HotSpot.

What do you think?

thanks
- Ioi

On 4/14/20 12:28 PM, Liu, Xin wrote:
> Hi, Chris,
>
> Your project is very interesting. Nice work! I've starred it.
> Ioi also show me some useful flags I don't know that. thanks!
>
> Hi, Ioi,
> I got some comments inline.
>
> On 4/12/20, 5:22 PM, "Ioi Lam" <ioi.lam at oracle.com> wrote:
>
>      CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>      
>      
>      
>      Hi Xin,
>      
>      Thanks for the feedback. It's very helpful.
>      
>      On 4/12/20 12:59 AM, Liu, Xin wrote:
>      > Hi, Ioi,
>      >
>      > I'm recently refactoring intrinsics options for compiler[1]. I introduced a new option type called "intrinsics".  The biggest trouble was that I had to modify multiple places.  I found your patch beautifully replaces jvmConstraintList.* &jvmFlagRangeList.* with some macros in jvmFlag.inline.hpp. great!
>      >
>      > I have some questions or suggests.
>      >
>      > 1. generate a document of options
>      > There're hundreds of JVM options.  It's hard to learn them by heart. I feel developers uses globals.hpp and sub header files as a de factor manual.
>      > if you break up the monolithic header, which is good,  I think developers deserve a manual.
>      > Off the top of my head, I think you can provide a mini tool, which consume a header aggerates all sub-header files and automatically generates a manual.
>      
>      Even today the flags are scattered around in various *.hpp files. There
>      are already ways of finding information about the all the flags:
>      
>           java -XX:+PrintFlagsWithComments   (debug build only)
>           java -XX:+PrintFlagsInitial
>           java -XX:+PrintFlagsFinal
>           java -XX:+PrintFlagsRanges
>      
>      These flags will continue to work after my patch. Would these be sufficient?
>      
>      Also, you can use this to look for a flag:
>      
>           find . -name \*.hpp | xargs grep FLAG.*SplitIfBlocks
>      
>      > 2. change FLAG_RANGE and FLAG_CONTRAINT to optional parameters of PRODUCT_FLAG.
>      > If you use variadic macro, it's possible to select RANGE or CONTRAINT or NIL from __VA_ARGS__.
>      > I used to propose it before. https://cr.openjdk.java.net/~xliu/8230669/00/webrev/src/hotspot/share/utilities/macro_overloading.hpp.html
>      >
>      > I think it's a good idea because it make the declaration section pure.
>      > If we replace those macros eg. PRODUCT_FLAG with C++ classes of same names, it's very easy to write a standalone tool to generate a manual of JVM options from header files, right?
>      
>      That's a good suggestion. I've uploaded an updated version of the design
>      that uses __VA_ARGS__
>      
>      http://cr.openjdk.java.net/~iklam/design/jvm_flags_overhaul/flags.hpp
>      http://cr.openjdk.java.net/~iklam/design/jvm_flags_overhaul/flags.cpp
>      
>      To see how the macros work, try doing this, and look at the file flags.ii
>      
>           gcc -DPRODUCT -O3 -o flags -save-temps flags.cpp && ./flags
>           gcc -g -o flags_g -save-temps flags.cpp && ./flags_g
>      
> --
> Oh, you use overloaded constructors to select optional parameters.
> Interesting, I didn't come up with that idea.
> --
>      I think we can try different designs using the above as a template. When
>      we have a more concrete design I can implement it in the VM.
>      
>      Now the declaration is:
>      
>           PRODUCT_FLAG(bool,     SplitIfBlocks, true, JVMFlag::DEFAULT,
>                                  "Clone compares and control flow through merge "
>                                  "points to fold some branches");
>      
>           DEVELOP_FLAG(intx,     FreqCountInvocations, 1, JVMFlag::DEFAULT,
>                                  "Scaling factor for branch frequencies",
>                                  FLAG_RANGE(1, max_intx))
>      
>           PRODUCT_FLAG(intx,     NodeLimitFudgeFactor, 2000, JVMFlag::CONSTRAINT,
>                                  "Fudge Factor for certain optimizations",
>      FLAG_CONSTRAINT(NodeLimitFudgeFactorConstraintFunc,
>                                  JVMFlag::AfterErgo))
>      
>           PRODUCT_FLAG(intx,     AliasLevel, 3, JVMFlag::DEFAULT,
>                                  "0 for no aliasing, 1 for .... split, "
>                                  "2 for class split, 3 for unique instances",
>                                  FLAG_RANGE(0, 3),
>      FLAG_CONSTRAINT(AliasLevelConstraintFunc,
>                                  JVMFlag::AfterErgo))
>      
>      and the implementation:
>      
>           DEFN_PRODUCT_FLAG(SplitIfBlocks);
>           DEFN_DEVELOP_FLAG(FreqCountInvocations);
>           DEFN_PRODUCT_FLAG(NodeLimitFudgeFactor);
>           DEFN_PRODUCT_FLAG(AliasLevel);
>      
>      My goal is still to put all the information about each flag in a single
>      place (the hpp file), so I need to pass the info to the cpp files using
>      an inline function:
>      
>         inline const ModifiableFlag<FLAG_TYPE_##name> init_##name() { \
>           return ModifiableFlag<FLAG_TYPE_##name>( \
>             JVMFlag::TYPE_##type, #name, attr, &name, __VA_ARGS__); \
>         }
>      
>      But the problem is this inline function could theoretically allocate the
>      instance on the stack, so I cannot save its address into the _head of
>      the global flags list. As I result, I needed to split the _next pointer
>      from JVMFlag into AbstractFlagLink::_link.
>      
>      If anyone has a better suggestion, I would be happy to try it out.
> --
> You can't get addresses of temps, but you can get addresses of static variables.  Is this trick useful for your aim?
>
> Eg.
>         inline const ModifiableFlag<FLAG_TYPE_##name>* init_##name() { \
>           static ModifiableFlag<FLAG_TYPE_##name>( \
>             JVMFlag::TYPE_##type, #name, attr, &name, __VA_ARGS__) t; \
>             Return &t;                                                                                              \
>         }
>
> Maybe Linkedlist isn't your only choice. I use enum to count how many options JVM will have and allocate a static array.
> In this way, I can even sort the array and use binary-search.
> Check out line #89 of https://cr.openjdk.java.net/~xliu/8151779/00/webrev/src/hotspot/share/compiler/intrinsics.cpp.html
> --
>     
>      > 2. There's a TCL script.  Isn't C Processor powerful enough for it?  I don't know TCL, I think an external script will bring dependency and learning  burden.
>      >
>      > 3. let's say we choose a script generator. It looks like those globals_xxx.cpp files are auto-gen files.
>      > Will you consider to untrack those generated files in the repo?
>      
>      The whole purpose of the TCL script is to convert the old style macros
>      in the hpp files into the new coding style. That way I can keep track of
>      changes in the upstream repo, and also easily change the design of the
>      new code. The script is used by me only (that's why it's messy). It's
>      not used during the build and will not be needed in the final patch.
>
>      The globals_xxx.cpp files will be checked into the repo. Afterwards, to
>      add/remove flags, you would need to edit the xxx_globals.hpp/cpp files
>      directly.
>      
>      As mentioned in the design doc, my goal is to avoid C++ code generation
>      as many people are against it.
>      
>      Personally, I prefer code generation. I think that can result in a
>      smaller binary footprint that will start up faster, but the delta is
>      probably negligible. We can probably leave that for the future. As long
>      as we use the macros in a consistent manner, we can easily extract the
>      flag definitions and put them into external input files that are
>      processed by a C++ code generator.
> ---
> But your current webrev leads to have a codegen path.
>
> You have to iterate all options at least twice, right? one is in header file, the other one is in the cpp file.
> The list is long and the order doesn't matter. I can't imagine you will check in those hpp/cpp pairs.  It's very natural to have a script like tcl or m4 to generate cpp from hpp, right?
> There was a high-order macro RUNTIME_FLAGS in previous globle.hpp. That's why it avoids to iterate options again & again. C-preprocessor did the "codegen" job by expanding macros.
>
> Why do you ditch that design? it still can archive all your design goals.
> ---
>      > 4. Probably, it's not a bad idea to still use product/develop instead of PRODUCT_FLAG, DEVELOP_FLAG.
>      > Those options will be read everyday by developers, I feel lower-case names are more friendly.
>      
>      I don't have a strong preference, but I think that adding "product"
>      globally (even as a macro) may cause problems with code like this:
>      
>           static int product(int a, int b) {return a*b;}
>      
>      as the C pre-processor would try to expand "product".
> --
> In previous globals.hpp, 'product' is just a macro parameter.  That's why current implementation doesn't have any conflict with the function product.
> --
>
>      Thanks
>      - Ioi
>      
>      
>      >
>      > [1]: https://cr.openjdk.java.net/~xliu/8151779/00/webrev/
>      >
>      > Thanks,
>      > --lx
>      >
>      > On 4/9/20, 5:08 PM, "hotspot-dev on behalf of Ioi Lam" <hotspot-dev-bounces at openjdk.java.net on behalf of ioi.lam at oracle.com> wrote:
>      >
>      >      CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>      >
>      >
>      >
>      >      The code for specifying HotSpot command-line flags used to be simple. At
>      >      the very beginning it was something like
>      >
>      >           #define ALL_FLAGS(develop, develop) \
>      >              product(int, SomeFlag, def_value, "some doc") \
>      >              develop(int, MoreFlag, def_value, "more doc") \
>      >
>      >      but like all good ideas, it has gradually degraded into a mess [1][2].
>      >      It's now very difficult to add any new functionality (e.g., add
>      >      "manageable" annotations to the flags [3]).
>      >
>      >      I think it's time for a complete rewrite :-)
>      >
>      >      My proposal is here. Please comment if it's a good or bad idea.
>      >
>      >      https://wiki.openjdk.java.net/display/HotSpot/HotSpot+Command-Line+Flags+Overhaul+-+Design+Doc
>      >
>      >
>      >
>      >      tldr; ---
>      >
>      >
>      >      OLD:
>      >
>      >         product(intx, AliasLevel, 3,                                          \
>      >                 "0 for no aliasing, 1 for oop/field/static/array split,
>      >      "         \
>      >                 "2 for class split, 3 for unique
>      >      instances")                      \
>      >                 range(0, 3)
>      >      /*optional*/                                         \
>      >                 constraint(AliasLevelConstraintFunc,AfterErgo) /*optional*/
>      >           \
>      >
>      >
>      >      NEW:
>      >
>      >         PRODUCT_FLAG(intx,   AliasLevel, 3, JVMFlag::RANGE | JVMFlag::CONSTRAINT,
>      >                              "0 for no aliasing, 1 for oop/field/static/array
>      >      split, "
>      >                              "2 for class split, 3 for unique instances");
>      >          // optional
>      >          FLAG_RANGE(         AliasLevel, 0, 3);
>      >          // optional
>      >          FLAG_CONSTRAINT(    AliasLevel, (void*)AliasLevelConstraintFunc,
>      >      JVMFlag::AfterErgo);
>      >
>      >
>      >
>      >
>      >      [1]
>      >      http://hg.openjdk.java.net/jdk/jdk/file/7af6364e1792/src/hotspot/share/runtime/globals.hpp#l115
>      >      [2]
>      >      http://hg.openjdk.java.net/jdk/jdk/file/7af6364e1792/src/hotspot/share/runtime/flags/jvmFlag.cpp#l635
>      >      [3] https://bugs.openjdk.java.net/browse/JDK-7123237
>      >
>      >      Thanks
>      >      - Ioi
>      >
>      >
>      
>      
>



More information about the hotspot-dev mailing list