eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 722 forks source link

Supporting Hotspot Large Page Options #8671

Open AlenBadel opened 4 years ago

AlenBadel commented 4 years ago

Supporting Hotspot Large Page Options

Goals

  1. To allow ease of migration from OpenJDK + Hotspot to OpenJDK + OpenJ9, this activity will focus implementing support for Hotspot large page options on OpenJ9

  2. There are a number of inconsistencies with the existing large page -Xlp options (-Xlp, -Xlp<size>, -Xlp:objectheap, and -Xlp:codecache) this issue will resolve these inconsistencies. These inconsistencies will be resolved by:

    • Identifying and fix current Issues with -Xlp options.
    • Deprecate the existing -Xlp large page options.
    • Implement new paging options that are backwards compatible with -Xlp options allowing the -Xlp options to be mapped to the new options until their removal, while preserving legacy behavior.

Parent Issue: https://github.com/eclipse/openj9/issues/4930

Design

Design Doc: https://ibm.box.com/s/rys4fllji6ji0h1a6tvw79j16w8cum5g

Implementation

Parsing

Currently, all -Xlp options parsing is done within the CodeCache[1], and ObjectHeap[2] somewhat redundantly. This needs to be removed in favor for a centralized solution, where the options are parsed within the VM rather than each component that they affect. Each component should only need to read the parsed results within shared data structure, and configure large pages. Thus the parsing code in [1], and [2] will be removed in favor for a centralized VM implementation.

Centralized Parsing

The solution to this issue will be centralized parsing. The VM will parse all large command line arguments(-XX, & -Xlp options) within jvminit.c[3], and save the net result into shared data structure. The net parsing result consists of the combined affect of all active large page options, neglecting those that are overwritten due to precedence rules. The parsing result will be stored within a new struct inside J9JavaVM[4]. We can name this structure J9LargePageOptionsInfo.

J9LargePageOptionsInfo will consist of the following:

BOOLEAN isEnabledForCodeCache /* Enabled LP on CodeCache */
BOOLEAN isEnabledForObjectHeap /* Enabled LP on ObjectHeap */
UDATA pageSizeForCodeCache /* Specified LP size on CodeCache */
UDATA pageSizeForObjectHeap  /* Specified LP size on ObjectHeap */
J9LargePageCautionLevel lpCautionLevel /* Specify if Warnings, Errors, or Neither have been enabled */
UDATA pageTypeForObjectHeap /* Z/OS Only: Specify if user desires a specific page type on ObjectHeap*/

Where J9LargePageCautionLevel is an enum to specify the following states:

J9CautionUnset /* User has not enabled Warnings, or Errors */
J9CautionWarning /* Enabled Warnings */
J9CautionError /* Enabled Errors */

and pageTypeObjectHeap can be the following:

J9PORT_VMEM_PAGE_FLAG_PAGEABLE_PREFERABLE /* User does not specify to explicitly use a page type. */
J9PORT_VMEM_PAGE_FLAG_PAGEABLE /* User explicitly asks for pageable large pages */
J9PORT_VMEM_PAGE_FLAG_FIXED /* User explicitly asks for fixed large pages */

Populating J9LargePageOptionsInfo

Specifically, this section will focus on the implementation of parsing all -XX, and -Xlp large page options within jvminit.c[3]. This is an area of discussion, since we have two possible implementations.

Firstly, to understand what options are passed in and their position among the user arguments each option variation will need to be searched and consumed[10]. The macro FIND_AND_CONSUME will return the right-most index of an option and if a match is not found the index will return -1. The option will also be consumed if it is found. FIND_AND_CONSUME will be called for each option that is being supported, including all newly proposed -XX options and -Xlp options.

Since each -XX option relates to only a specific subset of J9LargePageOptionsInfo, the implementation can be partitioned to focus on one subset at a time.

An example of this to analyze all options that enable, or disable large pages. Namely these are -XX:[+/-]UseLargePages, -XX:[+/-]UseLargePagesCodeCache, -XX:[+/-]UseLargePagesObjectHeap, -Xlp(-Xlp, -Xlp<size>, -Xlp:[codecache/objectheap].

/* -XX:[-/+]UseLargePages[/CodeCache/ObjectHeap] */
{
    IDATA argUseLargePagesEnable = FIND_AND_CONSUME_ARG(EXACT_MATCH, '-XX:+UseLargePages', NULL);
    IDATA argUseLargePagesDisable = FIND_AND_CONSUME_ARG(EXACT_MATCH, '-XX:-UseLargePages', NULL);
    IDATA argUseLargePagesCodeCacheEnable = FIND_AND_CONSUME_ARG(EXACT_MATCH, '-XX:+UseLargePagesCodeCache', NULL);
    IDATA argUseLargePagesCodeCacheDisable = FIND_AND_CONSUME_ARG(EXACT_MATCH, '-XX:-UseLargePagesCodeCache', NULL);
    IDATA argUseLargePagesObjectHeapEnable = FIND_AND_CONSUME_ARG(EXACT_MATCH, '-XX:+UseLargePagesObjectHeap', NULL);
    IDATA argUseLargePagesObjectHeapDisable = FIND_AND_CONSUME_ARG(EXACT_MATCH, '-XX:-UseLargePagesObjectHeap', NULL);

    /* Simplify to Codecache, and ObjectHeap components */
    argUseLargePagesCodeCacheEnable = OMR_MAX(argUseLargePagesCodeCacheEnable, argUseLargePagesEnable);
    argUseLargePagesCodeCacheDisable = OMR_MAX(argUseLargePagesCodeCacheDisable, argUseLargePagesDisable);
    argUseLargePagesObjectHeapEnable = OMR_MAX(argUseLargePagesObjectHeapEnable, argUseLargePagesEnable);
    argUseLargePagesObjectHeapDisable = OMR_MAX(argUseLargePagesObjectHeapDisable, argUseLargePagesDisable);

    /* Apply -Xlp mapping */
    argUseLargePagesCodeCacheEnable = OMR_MAX(argUseLargePagesCodeCacheEnable, argXlpEnableLargePagesCodeCache);
    argUseLargePagesObjectHeapEnable = OMR_MAX(argUseLargePagesObjectHeapEnable, argXlpEnableLargePagesObjectHeap);

    /* Set Lp Status Flags */
    lpInfo->isEnabledForCodeCache = argUseLargePagesCodeCacheEnable > argUseLargePagesCodeCacheDisable;
    lpInfo->isEnabledForObjectHeap = argUseLargePagesObjectHeapEnable > argUseLargePagesObjectHeapDisable;
}

Parsing -Xlp options A contrary to the preceding -Xlp ruins the party, since it may impact all members at the same time. Hence it's best to parse -Xlp, -Xlp<size>, -Xlp:[codecache/objectheap]:pagesize=<size>,[strict/warn],[pageable/nonpageable] ahead of parsing -XX options.

Xlp options will be parsed together. A set of front-facing (-XX large page compatabile) arguments can be used to translate and integrate the xlp options into how we parse the -XX large page options. The Xlp options can be parsed and simplified to the following variables.

IDATA argXlpEnableLargePagesCodeCache = -1;  // Index of right-most Xlp option that enables large pages on the codecache
IDATA argXlpEnableLargePagesObjectHeap = -1; // Index of right-most Xlp option that enables large pages on the objectheap

IDATA argXlpLargePageSizeInBytesCodeCache = -1; // Index of right-most Xlp option specifies a large page size for the codecache
UDATA xlpLargePageSizeCodeCache = -1;           // Value of that page size
IDATA argXlpLargePageSizeInBytesObjectHeap = -1; // Index of right-most Xlp option specifies a large page size for the objectheap
UDATA xlpLargePageSizeObjectHeap = -1;          // Value of that page size

IDATA argXlpPageWarningsEnable = -1;            // Index of right-most Xlp option specifies a "warn"
IDATA argXlpPageErrorsEnable = -1;            // Index of right-most Xlp option specifies a "strict"

#if defined(J9ZOS390)
IDATA argXlpObjectHeapPageType = -1;            // Index of right-most Xlp option specifies a [non]pageable
IDATA xlpObjectHeapPageType = -1;               // The value of non[pageable]
#endif

Specifics of Xlp, and -Xlp<size> are parsed using straight forward memory mapping. -Xlp:[codecache/objectheap]:pagesize=<size>,[strict/warn],[pageable/nonpageable] will need to be parsed by a sub option parser. Thankfully one already exists within the GC[9]. Since -Xlp parsing can be removed from the GC, the xlpSubOptionsParser can be moved to jvminit.c and used to parse any combination of this option.

Similarly, there will be isolated code to handle and populate other members of this instance. Including -XX:LargePageInBytes[/CodeCache/ObjectHeap]=<size>, -XX:[+/-]LargePageWarnings and -XX:[+/-]LargePageErrors, and -XX:zOSLargePagesObjectHeapType=<type>

isEnabledForCodeCache This member will be enabled only when an option that enables large pages on the codecache is either on the right of an option to disable it, or in absence of such option. As shown in the example above, we know the indices of each option variation this can easily be calculated.

isEnabledForObjectHeap The Above can be applied for the ObjectHeap as well.

pageSizeForCodeCache This will be set in accordance with the size specified on the right most index of any option that impacts the codecache page size. Namely, these are -Xlp<Size>, -Xlp:codecache:pagesize<size>, -XX:LargePageInBytes=<size> and -XX:LargePageInBytesCodeCache=<size>.

pageSizeForObjectHeap The Above can be applied for the ObjectHeap as well. Namely, the options that impact the objectheap pagesize are -Xlp<Size>, -Xlp:codecache:pagesize<size>, -XX:LargePageInBytes<size> and -XX:LargePageInBytesObjectHeap=<size>.

lpCautionLevel Warnings, will be enabled if options to enable it are to the right or in the absence of any option to disable it. The same applies to Errors. Options that enable Warnings include: -XX:+LargePageWarnings, -Xlp:[codecache/objectheap]:pagesize=<size>,warn. Options that disable Warnings include: -XX:-LargePageWarnings

Options that enable Errors include: -XX:+LargePageErrors, -Xlp:[codecache/objectheap]:pagesize=<size>,strict Options that disable Errors include: -XX:-LargePageErrors.

(Z/OS only) pageTypeForObjectHeap As, before in the absence of any option to specify the page type this will be set to J9PORT_VMEM_PAGE_FLAG_PAGEABLE_PREFERABLE, which will attempt for pageable variations but will fall back to fixed large pages. Otherwise, if an option that specifies the pagetype (-XX:zOSLargePagesObjectHeap=[pageable/nonpageable], or -XX:objectheap:pagesize=<size>,[non]pageable) exists the page type specified by the right-most index will be saved. (Pageable - J9PORT_VMEM_PAGE_FLAG_PAGEABLE, NonPageable - J9PORT_VMEM_PAGE_FLAG_FIXED).

(Common) Post-Parsing Implementation Details

In both options, the following checks will be necessary:

CodeCache Implementation

All Parsing will be removed from the codecache.[1] Removing unused Xlp JIT .nls macros. Modify, and move to VM .nls for use within the VM. Instead, large page configuration variables will be read from J9LargePageOptionsInfo.[5] How the codecache verifies, and configures the large pages will not change. [6]

ObjectHeap Implementation

All Parsing will be removed from the objectheap. [2] Removing unused Xlp GC .nls macros. Instead, large page configuration variables will be read from J9LargePAgeOptionsInfo. [7] How the objectheap verifies, and configures the large pages will not change, other than to accompany using data extracted from J9LargePAgeOptionsInfo. [8]

Related

An undocumented -Xlp option that manages the gc meta data page size is to be removed. -Xlp:gcmetadata will not be deprecated, rather it will be fully removed. All other -Xlp options, as were covered in this design will be deprecated.

Mainly this is just includes where the option is parsed, as all it's doing is overwriting the current gc metadata page size. https://github.com/eclipse/openj9/blob/491cde3d0b8383a30689fba244f547d6b9315fce/runtime/gc_modron_startup/mmparse.cpp#L815

Removing the capability of the user specifying the meta data page size, means that the metadata page size will always be set to use the system default page size of the system. https://github.com/eclipse/omr/blob/637f10724dd3de1d4845bd2bc38d9c25456941f3/gc/base/GCExtensionsBase.cpp#L129-L131

This validation would not be necessary. https://github.com/eclipse/omr/blob/637f10724dd3de1d4845bd2bc38d9c25456941f3/gc/base/GCExtensionsBase.cpp#L153-L156

Testing Implementation

CodeCache Testing is implemented here: XlpCodeCacheOptionsTestRunner.java

Objectheap Testing is implemented here: XlpOptionsTest.java

These CodeCache/ObjectHeap Tests use -verbose:sizes output to determine what page size was used to verify each large page size test case. As discussed, -verbose:sizes will need to be changed from displaying -Xlp attributes, to attributes that closely model the new -XX proposed options.

Particularity it's purposed that the verbose output be changed from

  -Xlp:objectheap:pagesize=<Page Size Used>,<Page Type Used on Z/OS>     large page size
                  available large page sizes:
                  < List of pages avaialble on obejct heap >
  -Xlp:codecache:pagesize=<Page Size Used>,<Page Type Used on Z/OS>  large page size for JIT code cache
                  available large page sizes for JIT code cache:
                  < List of pages avaialble on code cache>

To:

-XX:[+/-]UseLargePagesCodeCache
   Page Size Used: <Page Size Used By Code Cache>
   Page Type Used: <Page Type used on Z/OS>
   Available Page Sizes on Code Cache:
   <List of avaialble page sizes on the code cache>
-XX:[+/-]UseLargePagesObjectHeap
   Page Size Used: <Page Size Used by Object Heap>
   Page Type Used: <Page type used by Object Heap on Z/OS>
   Available Page Sizes on Object Heap:
   < List of avaialble page sizes on the object heap>

As each the codecache, and object heap test expects the output to be in the former -Xlp form, this needs to be adapted to conform with the verbose layout changes. The test will also need to be adapted to recongize expected warnings when warnings are enabled.

Summary of Changes

Doc Changes Description PR
Add Docs for new -XX Options TBD
Changes to -Xlp, -Xlp:codecache, and -Xlp:objectheap TBD
Test Changes Description PR
Add Tests for new -XX options TBD
Add j9vmemtests to verify omrvmem_find_valid_page_size & omrvmem_default_large_page_size_ex with OMRPORT_VMEM_PAGE_FLAG_PREFER_PAGEABLE https://github.com/eclipse/omr/pull/4918 https://github.com/eclipse/openj9/pull/8791
Related Changes Description PR
Enable XlpOptions, and XlpCodeCache tests https://github.com/eclipse/openj9/pull/8576
Expand XlpCodeCache tests to include variations https://github.com/eclipse/openj9/pull/8025

References and Footnotes

[1] https://github.com/eclipse/openj9/blob/f7c40c6c6ac97c5bdad13336ab2360f3efb9ff8b/runtime/compiler/control/J9Options.cpp#L1465-L1694 [2] https://github.com/eclipse/openj9/blob/c560cd75ea7ca9c9f2442d2eab7b1afa8a15af1e/runtime/gc_modron_startup/mmparse.cpp#L587-L670 [3] https://github.com/eclipse/openj9/blob/6a33c82664bdc5c0ac8860c85ab8fdcf18bee1ec/runtime/vm/jvminit.c#L1614 [4] https://github.com/eclipse/openj9/blob/491cde3d0b8383a30689fba244f547d6b9315fce/runtime/oti/j9nonbuilder.h#L4780 [5] https://github.com/eclipse/openj9/blob/f7c40c6c6ac97c5bdad13336ab2360f3efb9ff8b/runtime/compiler/control/J9Options.cpp#L1458-L1461 [6] https://github.com/eclipse/openj9/blob/f7c40c6c6ac97c5bdad13336ab2360f3efb9ff8b/runtime/compiler/control/J9Options.cpp#L1696-L1784 [7] https://github.com/eclipse/openj9/blob/c560cd75ea7ca9c9f2442d2eab7b1afa8a15af1e/runtime/gc_modron_startup/mmparse.cpp#L583-L584 [8] https://github.com/eclipse/openj9/blob/c560cd75ea7ca9c9f2442d2eab7b1afa8a15af1e/runtime/gc_modron_startup/mmparse.cpp#L674-L719 [9] https://github.com/eclipse/openj9/blob/8e50c296bba256a5280e4529a6995b3e5e200b9e/runtime/gc_modron_startup/mmparse.cpp#L356-L357 [10] Consumed Arguments: Unique command line arguments must be marked as consumed for control purposes. Duplicate arguments that are later re-defined are automatically consumed. Each option will have at most one unconsumed argument that needs to be consumed when that argument is processed.

AlenBadel commented 4 years ago

@dmitripivkine Would you have any remaining concerns?

@joransiu I would appreciate if you could give a quick approval on the Z/OS related content.

joransiu commented 4 years ago

The z/OS proposal for handling -XX:+UseLargePages and -XX:LargePageSizeInBytes= looks good to me. Thanks!

dmitripivkine commented 4 years ago

As top level description item it looks reasonable for me.

AlenBadel commented 4 years ago

@pshipton @dmitripivkine The official docs for -Xlp does not specify precedence rules. Within the objectheap implementation, and tests we assume that if -Xlp is passed with any other arrangement of -Xlp options it is always ignored. This happens even if it is the right-most option. The only time it is a valid option is if it is passed without any other -Xlp option.

Was this intended? If so then it needs to be documented since it doesn't follow logical precedence.

E.g -Xlp32M -Xlp

https://github.com/eclipse/openj9/blob/c560cd75ea7ca9c9f2442d2eab7b1afa8a15af1e/runtime/gc_modron_startup/mmparse.cpp#L587-L590

https://github.com/eclipse/openj9/blob/3dab54528080af0731e11858403a37ed962728d8/test/functional/VM_Test/src/j9vm/test/xlp/XlpOptionsTestRunner.java#L172-L177

DanHeidinga commented 4 years ago

I wrote up a proposal on how to handle these options looking at what we'd do today if they were completely new options being added. We obviously don't have a blank slate so we need to figure out how to handle both the new and old options.

I've spent a lot of time thinking about this since you slacked me about it and I had an insight over the weekend. We'd never add the -Xlp:... option forms today in that format. They would be split into individual -XX:[+-] options for easier parsing and clearer intent. Allowing each component to examine the ones specific to it.

That leads to the key insight - the old options should be mapped to the new form rather than trying to make the new fit the old.

See https://github.com/eclipse/openj9/pull/7476#issuecomment-637601836 for my description of how we should do this.

For you question above:

Was this intended? If so then it needs to be documented since it doesn't follow logical precedence.

I think this falls out automatically with the new approach: -Xlp -> -XX:+UseLargePages -Xlp4k -> -XX:LargePageSizeInBytes=4k

and both of those are valid regardless of the order they occur in.

AlenBadel commented 4 years ago

One possible change of legacy I see happening is related to a decision we've made in the last design of -XX:+UseLargePages on z/OS. Using -Xlp will always request 1M fixed large pages instead of pageable. We've made the decision that pageable large pages being more performant and should be used within -XX:+UseLargePages. We have a number of users that depend on the legacy behaviour of -Xlp since there are a number of functional trade-offs between fixed and pageable on z/OS.

For now, I suggest we have a special version of -Xlp that is not internally mapped to -XX:+UseLargePagesObjectHeap until EOL of Java 8 where the option is set to be deprecated.

All other Xlp* variants can easily be mapped.

AlenBadel commented 4 years ago

From your description it looks like we need the following options.

-XX:[+/-]UseLargePages
-XX:[+/-]UseLargePagesObjectHeap
-XX:[+/-]UseLargePagesCodeCache
-XX:LargePageSizeInBytes=<Size>
-XX:CodeCacheLargePageSizeInBytes=<Size>
-XX:ObjectHeapLargePageSizeInBytes=<Size>

I suspect we're missing an option, or sub-option that allows us to specify [non]pageable on z/OS. Looking back at -Xlp, if we had an option to toggle page type we could easily map -Xlp to: -XX:+UseLargePagesObjectHeap -XX:LargePageType=nonpageable. I'm not sure how practical a z/OS only option sounds.

DanHeidinga commented 4 years ago

I'd prefer to avoid suboptions when possible, so -XX:LargePageType=nonpageable would be -XX:[+-]UseNonpageableLargePages.

And that seems like a reasonable way to handle -Xlp on zOS

DanHeidinga commented 4 years ago

Are all three sizes needed? Do we have any evidence of users specifying different large page sizes for the object heap vs code cache?

AlenBadel commented 4 years ago

Are all three sizes needed? Do we have any evidence of users specifying different large page sizes for the object heap vs code cache?

I'm not sure. @pshipton Could you chime in? From the research I've done there doesn't seem to be such a thing on Hotspot - perhaps for good reason?

pshipton commented 4 years ago

I don't know what options are used by customers. I suspect some customers set large pages for one or the other but not both, but not sure how I know this.

We already support options to set the objectheap and codecache sizes separately. We could get away with not adding -XX:LargePageSizeInBytes=<Size> since this is just a shortcut for setting both the other options, except its needed for compatibility.

DanHeidinga commented 4 years ago

The options should either start with the 'ObjectHeap|CodeCache` or end with that. Current proposal changes form:

-XX:[+/-]UseLargePages
-XX:[+/-]UseLargePagesObjectHeap
-XX:[+/-]UseLargePagesCodeCache
-XX:LargePageSizeInBytes=<Size>
-XX:CodeCacheLargePageSizeInBytes=<Size>
-XX:ObjectHeapLargePageSizeInBytes=<Size>

Better to be consistent and always prepend or append it so:

-XX:[+/-]UseLargePages
-XX:[+/-]ObjectHeapUseLargePages
-XX:[+/-]CodeCacheUseLargePages
-XX:LargePageSizeInBytes=<Size>
-XX:CodeCacheLargePageSizeInBytes=<Size>
-XX:ObjectHeapLargePageSizeInBytes=<Size>

or

-XX:[+/-]UseLargePages
-XX:[+/-]UseLargePagesObjectHeap
-XX:[+/-]UseLargePagesCodeCache
-XX:LargePageSizeInBytes=<Size>
-XX:LargePageObjectHeapSizeInBytes=<Size>
-XX:LargePageCodeCacheSizeInBytes=<Size>
AlenBadel commented 4 years ago

I've went ahead and posted the design doc of the current paging options, as well as the proposed additions we've discussed.

See https://github.com/eclipse/openj9/issues/8671#issue-571666508

@dmitripivkine @DanHeidinga @pshipton Feel free to review the behaviour and let me know if anything is unclear or inaccurate.

pshipton commented 4 years ago

lgtm

AlenBadel commented 4 years ago

@pshipton Is it right to assume that the irregularity described in https://github.com/eclipse/openj9/issues/8671#issuecomment-637582166 is a bug and that the option precedence for -Xlpshould follow all other -Xlp* options?

DanHeidinga commented 4 years ago

Thanks for pulling this together @AlenBadel.

A couple of comments:

Old Option Mapping
-Xlp -XX:+UseLargePagesObjectHeap
-Xlp (z/OS) -XX:+UseLargePagesObjectHeap -XX:+UseNonpageableLargePages

For the question about Precedence rules, we don't need to worry about "last applicable option wins". The + & - forms can be applied left to right and the final configuration read. Same for the sizes. This is partly why I prefer the -XX:[+-] forms.

AlenBadel commented 4 years ago

Thanks for the review and clarification @DanHeidinga. I've made the changes.

pshipton commented 4 years ago

Is it right to assume that the irregularity described in https://github.com/eclipse/openj9/issues/8671#issuecomment-637582166 is a bug

Certainly that makes things much easier than trying to replicate the current behavior once we start mapping -Xlp options to -XX: options. However it could mean a change in behavior for a customer when updating, so we should make sure we document this in the release notes.

dmitripivkine commented 4 years ago

Is it right to assume that the irregularity described in #8671 (comment) is a bug

Certainly that makes things much easier than trying to replicate the current behavior once we start mapping -Xlp options to -XX: options. However it could mean a change in behavior for a customer when updating, so we should make sure we document this in the release notes.

I believe this is intentional. -Xlp supposed to be ignored if -Xlp<size> is provided. It is like "I want large pages and size of large page should be <size>". I believe this is kind of "translation" of -XX:+UseLargePages -XX:LargePageSizeInBytes=<Size>

gita-omr commented 4 years ago

What should happen if -XX:LargePageSizeInBytes=<Size> -XX:+UseLargePages is specified?

AlenBadel commented 4 years ago

What should happen if -XX:LargePageSizeInBytes=<Size> -XX:+UseLargePages is specified?

Logically, -XX:LargePageSizeInBytes=<Size> should be ignored. It doesn't make sense that an argument be passed before the feature is enabled. I assume there's a global OpenJ9 option convention rule to address this.

I believe this is intentional. -Xlp supposed to be ignored if -Xlp is provided

If was intentional, it wasn't documented. Let's agree to treat all options equally, and not have -Xlp overwritten. As Peter mentioned, it should a reasonable action as long as it's properly documented and we've marked these options for deprecation.

dmitripivkine commented 4 years ago

I believe different options should be context independent (order irrelevant). My point was in this context -Xlp (enable LP) and -Xlp<size> (specify size for LP) should be treated at two different options (not a two forms of the same option). if somebody specifies (on AIX) -Xlp64k -Xlp LP size selected should be 64k, not 16m default and order does not matter: -Xlp -Xlp64k should be equivalent.

pshipton commented 4 years ago

I agree with Dmitri. -XX:LargePageSizeInBytes=<Size> -XX:+UseLargePages should use <Size>, not ignore the size because it comes first.

AlenBadel commented 4 years ago

Agreed, options will be proceed no matter of the order, unless they are overwritten, or disabled by an option at a later index.

I certainly misspoke. We wouldn't need any special cases or precedence for -Xlp. Ultimately it will be mapped to -XX:+UseLargePages, and any option bearing a size argument (-Xlp<size>, -Xlp:[codecache/objectheap]) will effectively dictate the page size used. All legacy is preserved without needing any special cases.

AlenBadel commented 4 years ago

I've updated the doc to reflect the prior discussion. Namely the changes are:

AlenBadel commented 4 years ago

@DanHeidinga & @dmitripivkine Would you have any further concerns?

@joransiu, @zl-wang @andrewcraik Just as an FYI about the deprecation of the -Xlp options. The OpenJ9 docs outlined that we would deprecate only -Xlp, and -Xlp<size> after Java 8; keeping -Xlp:[codecache/objectheap] options. Here, we're proposing to deprecate all -Xlp* options starting on Java8, and users will ultimately see a warning when using these options.

andrewcraik commented 4 years ago

What is the plan with regard to still being able to tune the codecache and objectheap sizes independently for performance investigations etc? Are there equivalent options in the new scheme?

AlenBadel commented 4 years ago

What is the plan with regard to still being able to tune the codecache and objectheap sizes independently for performance investigations etc? Are there equivalent options in the new scheme?

I've linked a spreadsheet which provides explanations of the current -Xlp option behaviour, the new -XX options proposed, and how the older options will be mapped to the new proposed options internally. See: https://github.com/eclipse/openj9/issues/8671#issue-571666508

The proposed addition to of -XX:LargePageSizeInBytesCodeCache=<size>, and -XX:LargePageSizeInBytesObjectHeap=<size> will allow users to independently set the codecache, and objectheap sizes respectively. Keeping in mind that large pages will need to be enabled as well with (-XX:+UseLargePages[Codecache/ObjectHeap]) before it would take effect. Hence why something like -Xlp:codecache:pagesize=<size> is mapped as -XX:+UseLargePagesCodeCache -XX:LargePageSizeInBytesCodeCache=<size>.

dmitripivkine commented 4 years ago

Keeping in mind that large pages will need to be enabled as well with (-XX:+UseLargePages[Codecache/ObjectHeap]) before it would take effect.

I am not sure this is correct behaviour. Why if for example -XX:LargePageSizeInBytesObjectHeap=<size> is requested it requires to use another option -XX:+UseLargePagesObjectHeap to be "enabled"? It is self-complete option. I believe it should take effect regardless

AlenBadel commented 4 years ago

From my understanding this was an OpenJ9 option convention.

As @DanHeidinga mentioned in https://github.com/eclipse/openj9/pull/7476#issuecomment-637601836

The current -Xlp:... options would look very different if they were being developed today. Each sub-option would be its own -XX:[+-] form. So -Xlp:objectheap:pagesize=2G would be written as -XX:+UseLargePagesObjectHeap -XX:LargePageSizeInBytes=2G

AlenBadel commented 4 years ago

FYI @mstoodle @0xdaryl. Feel free to chime in on this proposal.

DanHeidinga commented 4 years ago

Why if for example -XX:LargePageSizeInBytesObjectHeap= is requested it requires to use another option -XX:+UseLargePagesObjectHeap to be "enabled"? It is self-complete option. I believe it should take effect regardles

We're mapping to Hotspot's behaviour, which as far as I can tell from -XX:+PrintFlagsFinal, doesn't enable large pages unless explicitly requested.

Our legacy size options become the equivalent of an enable + the setting the size.

andrewcraik commented 4 years ago

Keeping in mind that large pages will need to be enabled as well with (-XX:+UseLargePages[Codecache/ObjectHeap]) before it would take effect.

I am not sure this is correct behaviour. Why if for example -XX:LargePageSizeInBytesObjectHeap= is requested it requires to use another option -XX:+UseLargePagesObjectHeap to be "enabled"? It is self-complete option. I believe it should take effect regardless

If we do require the enablement option to make the page size setting take effect then I would hope we could generate a warning to help people avoid falling into the trap of setting the size but not setting the enablement...

dmitripivkine commented 4 years ago

Why if for example -XX:LargePageSizeInBytesObjectHeap= is requested it requires to use another option -XX:+UseLargePagesObjectHeap to be "enabled"? It is self-complete option. I believe it should take effect regardles

We're mapping to Hotspot's behaviour, which as far as I can tell from -XX:+PrintFlagsFinal, doesn't enable large pages unless explicitly requested.

Our legacy size options become the equivalent of an enable + the setting the size.

Ok, I was under impression that Hotspot's option to set size enable feature implicitly

mstoodle commented 4 years ago

I appreciate all the attention being brought to this issue to ensure we build the right solution.

Since I was asked (and not because I think it really adds anything to this conversation), I support the direction @DanHeidinga has been recommending in this issue. I agree with trying to process options in a way consistent with how Hotspot processes its options. Policies like "setting the large page size also enables large pages" makes for complicated rules to reason about for both the JVM and for people (e.g. what's the rightmost large page enabling option versus the rightmost -XX:-UseLargePages). It's also hard to be complete and future proof.

Note that having full -XX:+PrintFlagsFinal support would help with (though not solve) the issue @andrewcraik raised. Warning messages seem helpful but can also unintentionally cause pain.

andrewcraik commented 4 years ago

I'm fine with maximum compatibility. A warning would be nice but I do see the point @mstoodle is making. If there is an option to warn about ignored parameters (which I think we already have) then that would satisfy me since I can just make sure to always run with that to validate any configurations I build with options.

pshipton commented 4 years ago

https://www.eclipse.org/openj9/docs/xxignoreunrecognizedxxcolonoptions/

AlenBadel commented 4 years ago

Great, if -XX:LargePageSizeInBytes, or any of it's variants are specified but the respective component's large page functionality has not been enabled, and -XX:-IgnoreUnrecognizedXXColonOptions is used we'll simply provide an error.

pshipton commented 4 years ago

and -XX:-IgnoreUnrecognizedXXColonOptions is used we'll simply provide an error.

No, they are recognized options. They should not cause any error, but be consumed without having any effect.

pshipton commented 4 years ago

To have warnings about invalid large page configurations we should add another option like -XX:+LargePageWarnings. Note the existing -Xlp:objectheap option supports [,strict|warn] options, and these should also have -XX: equivalents before deprecating -Xlp:objectheap. https://www.eclipse.org/openj9/docs/xlpobjectheap/

AlenBadel commented 4 years ago

To support strict, and warn we would need an additional two options. -XX:[+/-]LargePageWarnings(warn), and -XX:[+/-]LargePageErrors(strict) keeping precedence rules as If both strict and warn are specified, strict takes precedence.

Since these options will only impact the objectheap, would we want to name them -XX:[+/-]LargePageWarningsObjectHeap, and -XX:[+/-]LargePageErrorsObjectHeap similarly to https://github.com/eclipse/openj9/issues/8671#issuecomment-639693280?

Row 12 adds a new -XX:+UseNonpageableLargePages option and indicates it only applies to the Object heap.
    If it's only object heap, it should be renamed to be clear on that fact: -XX:+UseNonpageableLargePagesObjectHeap
DanHeidinga commented 4 years ago

Since these options will only impact the objectheap, would we want to name them -XX:[+/-]LargePageWarningsObjectHeap, and -XX:[+/-]LargePageErrorsObjectHeap similarly to #8671 (comment)?

Yes.

AlenBadel commented 4 years ago

Great. I've updated the design to include:

AlenBadel commented 4 years ago

-XX:[+/-]LargePageWarnings, and -XX:[+/-]LargePageErrors are better naming options because we ultimately need options that enable warnings, and errors for both the codecache and objectheap.

-XX:[+/-]LargePageWarnings

(ObjectHeap) Causes a warning message to be generated if large pages are requested but cannot be obtained. Also, this option will also cause a warning message if the user has set a large apge size (-XX:LargePageSizeInBytes, -XX:LargePageSizeInBytesObjectHeap), but has not enabled Large Pages (-XX:+UseLargePages, or -XX:+UseLargePagesObjectHeap). This option allows the VM to continue.

(CodeCache) Causes a warning message to be generated if the user has set a large page size(-XX:LargePageSizeInBytes, or -XX:LargePAgeSizeInBytesCodeCache), but large pages are not enabled (-XX:+UseLargePages, or -XX:+UseLargePagesCodeCache). This option allows the VM to continue.

-XX:[+/-]LargePageErrors (ObjectHeap) Causes an error message to be generated if large pages are requested but cannot be obtained. Also causes an error message if the user has set a large page size (-XX:LargePageSizeInBytes, -XX:LargePageSizeInBytesObjectHeap), but has not enabled Large Pages (-XX:+UseLargePages, or -XX:+UseLargePagesObjectHeap). This option causes the VM to end.

(CodeCache) Causes an error message to be generated if the user has set a large page size(-XX:LargePageSizeInBytes, or -XX:LargePAgeSizeInBytesCodeCache), but large pages are not enabled (-XX:+UseLargePages, or -XX:+UseLargePagesCodeCache). This option allows the VM to end.

An issue I see occurs when mapping these options to -XX:objectheap:pagesize=<size>,[strict/warn]. If the strict/warn sub-option is mapped to LargePageErrors, and LargePageWarnings respectively this will enable errors and warnings not only for the objectheap, but also the codecache. I don't see how enabling warnings for the codecache would be imperative for the user but we would be introducing additional warning, and error path that wasn't part of the legacy xlp behaviour. The question is if this is acceptable, or do we need objectheap variant options for warnings and errors.

gita-omr commented 4 years ago

An issue I see occurs when mapping these options to -XX:objectheap:pagesize=<size>,[strict/warn]. If the strict/warn sub-option is mapped to LargePageErrors, and LargePageWarnings respectively this will enable errors and warnings not only for the objectheap, but also the codecache. I don't see how enabling warnings for the codecache would be imperative for the user but we would be introducing additional warning, and error path that wasn't part of the legacy xlp behaviour. The question is if this is acceptable, or do we need objectheap variant options for warnings and errors.

It's a good point. But mapping is really an implementation issue, is not it? We don't have to map, just set a flag "warn about objectheap" when we see -XX:objectheap:pagesize=<size>,warnand set "warn about objectheap" flag as well as "warn about codecache" when we see XX:+LargePageWarnings

0xdaryl commented 4 years ago

Can someone explain the reason why there isn't currently a strict or warning option for large code pages?

pshipton commented 4 years ago

Unless somebody else has an explanation, I'd say nobody asked for it.

0xdaryl commented 4 years ago

If that's the only reason then for symmetry I suggest that support be added going forward.

AlenBadel commented 4 years ago

It's a good point. But mapping is really an implementation issue, is not it? We don't have to map, just set a flag "warn about objectheap" when we see -XX:objectheap:pagesize=<size>,warnand set "warn about objectheap" flag as well as "warn about codecache" when we see XX:+LargePageWarnings

We've been able to exactly map each -Xlp option, and sub-option to a newly created -XX: option. It would be great to continue doing so without adding a specialty parsing rule. This would prevent us from needing special parsing rules when someone decides to use -Xlp, and large page -XX options together.

If that's the only reason then for symmetry I suggest that support be added going forward.

Extending the question I asked earlier, would it sound clear to everyone that enabling [strict/warn] on -Xlp:codecache, would enable the same on the object-heap without the option being specified and vice versa. Otherwise I can always create three different variations for each warn, and error options. (Objectheap, CodeCache, and Both)

dmitripivkine commented 4 years ago

Unless somebody else has an explanation, I'd say nobody asked for it.

As I remember strict/warning sub options were added as a response to controversial requests: Some people uses the same java command line across multi platform environment so option value might be not applicable sometimes but they want execute it regardless and some people wants immediate stop. For details see RTC Jazz 93937