EgorBaranovEnjoysTyping commented on code in PR #13413:
URL: https://github.com/apache/ignite/pull/13413#discussion_r3958185376
##########
modules/ducktests/tests/ignitetest/services/utils/jvm_utils.py:
##########
@@ -17,25 +17,85 @@
This module contains JVM utilities.
"""
+import re
+
from ignitetest.services.utils.decorators import memoize
DEFAULT_HEAP = "768M"
-JVM_PARAMS_GC_G1 = "-XX:+UseG1GC -XX:MaxGCPauseMillis=100 " \
- "-XX:ConcGCThreads=$(((`nproc`/3)>1?(`nproc`/3):1)) " \
- "-XX:ParallelGCThreads=$(((`nproc`*3/4)>1?(`nproc`*3/4):1))
"
+GC_G1 = "G1"
+GC_PARALLEL = "PARALLEL"
+GC_SERIAL = "SERIAL"
+GC_Z = "ZGC"
+GC_SHENANDOAH = "SHENANDOAH"
+
+DEFAULT_GC = GC_G1
+
+# NOTE: these strings are interpolated into a shell command that is evaluated
on the remote
+# node (see IgniteSpec._jvm_opts and IgniteNodeSpec.command), which is what
makes the `nproc`
+# substitutions work. Consequently NO option here may contain spaces or quotes.
+_NPROC_THIRD = "$(((`nproc`/3)>1?(`nproc`/3):1))"
+_NPROC_THREE_QUARTERS = "$(((`nproc`*3/4)>1?(`nproc`*3/4):1))"
+
+# Garbage collector profiles. A profile is a mutually exclusive group: it both
selects the collector
+# and carries the tuning flags that are meaningful for it. Never mix flags
across profiles.
+GC_PROFILES = {
+ GC_G1: [
+ "-XX:+UseG1GC",
+ "-XX:MaxGCPauseMillis=100",
+ f"-XX:ConcGCThreads={_NPROC_THIRD}",
+ f"-XX:ParallelGCThreads={_NPROC_THREE_QUARTERS}",
+ "-XX:+UseStringDeduplication", # G1-only until JDK 18, hence part of
the profile
+ ],
+ GC_PARALLEL: [
+ "-XX:+UseParallelGC",
+ f"-XX:ParallelGCThreads={_NPROC_THREE_QUARTERS}",
+ # deliberately NO MaxGCPauseMillis: it flips ParallelGC into adaptive
pause-goal sizing
+ ],
+ GC_SERIAL: [
+ "-XX:+UseSerialGC",
+ ],
+ GC_Z: [
+ "-XX:+UseZGC", # product feature since JDK 15, no unlock flag needed
+ f"-XX:ConcGCThreads={_NPROC_THIRD}",
+ f"-XX:ParallelGCThreads={_NPROC_THREE_QUARTERS}",
+ ],
+ GC_SHENANDOAH: [
+ "-XX:+UseShenandoahGC", # product feature since JDK 15; OpenJDK only,
not Oracle JDK
+ f"-XX:ConcGCThreads={_NPROC_THIRD}",
+ f"-XX:ParallelGCThreads={_NPROC_THREE_QUARTERS}",
+ ],
+}
JVM_PARAMS_GENERIC = "-server -XX:+DisableExplicitGC -XX:+AlwaysPreTouch " \
"-XX:+ParallelRefProcEnabled -XX:+DoEscapeAnalysis " \
- "-XX:+OptimizeStringConcat -XX:+UseStringDeduplication"
+ "-XX:+OptimizeStringConcat"
+
+# Matches a collector selector like -XX:+UseZGC. Deliberately narrow: it must
not match
+# -XX:+DisableExplicitGC or -XX:+UseStringDeduplication.
+_GC_SELECTOR_PATTERN = re.compile(r"^-XX:([+-])(Use\w+GC)$")
+
+
+class MultipleGcSelectedError(Exception):
+ """
+ Raised when JVM options end up selecting more than one garbage collector.
+ """
-def create_jvm_settings(heap_size=DEFAULT_HEAP, gc_settings=JVM_PARAMS_GC_G1,
generic_params=JVM_PARAMS_GENERIC,
+def create_jvm_settings(heap_size=DEFAULT_HEAP, gc_settings=None,
generic_params=JVM_PARAMS_GENERIC,
gc_dump_path=None, oom_path=None, vm_error_path=None):
"""
Provides settings string for JVM process.
- param opts: JVM options to merge. Adds new or rewrites default values. Can
be list or string.
+ :param heap_size: value for both -Xmx and -Xms.
+ :param gc_settings: garbage collector options, see GC_PROFILES. Can be
list or string.
+ Defaults to the DEFAULT_GC profile.
+ :param generic_params: collector-independent options. Can be list or
string.
"""
+ gc_settings = GC_PROFILES[DEFAULT_GC] if gc_settings is None else
gc_settings
+
+ if isinstance(gc_settings, str):
Review Comment:
If this is string, then split it by whitespaces. otherwise just keep the
same value (e.g. None, list, dict), and on line 111 method '
'.join(gc_settings) could fail. So if it is a string always (otherwise it
fails).
So it should fail in case of non str value, or don't check it's string (or
directly assert it).
If I see isInstrance(val, str), then author asserts "It might be non-string"
##########
modules/ducktests/tests/ignitetest/services/utils/gc_params.py:
##########
@@ -0,0 +1,88 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module resolves the garbage collector to use from Globals.
+
+GC selection is mutually-exclusive group replacement: a collector and its
tuning flags travel together
+(see GC_PROFILES in jvm_utils). It therefore has to be chosen *before* the
default option list is
+assembled -- patching it afterwards via jvm_opts leaves two selectors in the
command line, because
+merge_jvm_settings overwrites per option, not per group.
+
+This is the single resolution point for the 'gc' global. Keep it that way.
+"""
+
+from ignitetest.services.utils.jvm_utils import DEFAULT_GC, GC_PROFILES
+
+GC_KEY_NAME = "gc"
+
+SERVER_ROLE = "server"
+CLIENT_ROLE = "client"
+
+
+def is_gc_configured(_globals: dict):
+ """
+ :param _globals: Globals parameters
+ :return: True if the run explicitly selects a garbage collector.
+ """
+ return GC_KEY_NAME in (_globals or {})
+
+
+def resolve_gc_settings(_globals: dict, role: str):
+ """
+ Gets garbage collector options from Globals. Three shapes are accepted:
+
+ {"gc": "ZGC"} -- both roles
+ {"gc": {"server": "ZGC"}} -- servers only,
clients keep the default
+ {"gc": {"server": "ZGC", "client": "SERIAL"}} -- per role
+ {"gc": {"server": ["-XX:+UseZGC", "-XX:SoftMaxHeapSize=2G"]}} -- raw
options, escape hatch
+
+ Profile names are case-insensitive. A missing role, or a missing 'gc' key,
yields the DEFAULT_GC
+ profile. A list value is used verbatim and bypasses profile validation --
that is the point of it.
+
+ :param _globals: Globals parameters
+ :param role: SERVER_ROLE or CLIENT_ROLE
+ :return: list of JVM options selecting and tuning the collector
+ """
+ configured = (_globals or {}).get(GC_KEY_NAME)
+
+ if configured is None:
+ return _profile(DEFAULT_GC)
+
+ if isinstance(configured, dict):
+ configured = configured.get(role)
+
+ if configured is None:
+ return _profile(DEFAULT_GC)
Review Comment:
Sorry, my fault. Looks like it really works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
