RFC 8236988 - Modular Design for JVM Flags
Ioi Lam
ioi.lam at oracle.com
Mon Apr 13 00:22:11 UTC 2020
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
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.
> 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.
> 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".
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