Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v7]
On Thu, 21 Dec 2023 10:01:04 GMT, Thomas Stuefe wrote: >>> > > What happens if we accidentally attempt to load a "real" static >>> > > library, which is also named *.a? Would dlopen() then crash? What would >>> > > happen? >>> >>> > I don't think the problem is with *.a . They would load as the default >>> > behaviour of the dlopen. It is only when the dlopen fails for *.so , we >>> > give another chance to check for .a file with the same name. >>> >>> No, what I meant, and what must be clarified before going forward with this >>> solution, is the following: >>> >>> * is _every_ `*.a` object on AIX loadable with `dlopen`, and will the >>> result be the same as when loading a `*.so` object >>> * or, if we present arbitrary `*.a` files to dlopen, is there a chance for >>> dlopen to crash or misbehave. >>> >>> Reason is that I was under the impression that *.a libraries are static >>> libraries and cannot be loaded dynamically. This is what you now try to do. >>> If we cannot safely answer this question, I would opt for a more narrow >>> solution by hard-wiring known alternative names. So, do the second *.a >>> attempt only for your `ibm_16_am.a` which you know works. That could also >>> be done in a reasonably maintainable manner. >>> >> In AIX, both static and dynamic libraries have *.a extension. And AIX also >> supports *.so files.Bascially shared objects in AIX have both *.a and *.so >> extension. Hence we need to implement this logic. >> If we try loading a static archive specifically ,how the dlopen would behave >> , that is something probably @JoKern65 can answer ? >> >> >>> > > Does this really have to be handled in the OpenJDK? What does J9 on AIX >>> > > do? Could this be done in a simpler way outside OpenJDK, e.g. by >>> > > providing an *.so variant of the library in question? Where does this >>> > > library come from? >>> >>> > I am not sure how J9 handles this. I would have to consult . >>> >>> J9 is Open Source, can't you just look? :) >> >> I did try comparing the file structures, and i do not see a similar file >> structure over there. >> I am unable to find the jvmTiAgent code and also os_aix file. So i am not >> sure which functions over there are doing the same functionality. You have >> any suggestion on how i can check and correlate ? >>> >>> > However as per current observation, this issue does not show up on >>> > Semuru. This issue is only happening on Adoptium. The team that release >>> > these file has always released *.a files which work fine for Semuru. >>> >>> I don't know what Semuru is. What is the context, is that a different VM? >>> Also OpenJDK? J9 derived? >> >> >> Semuru is J9 derived. > >> > > > What happens if we accidentally attempt to load a "real" static >> > > > library, which is also named *.a? Would dlopen() then crash? What >> > > > would happen? >> > >> > >> > > I don't think the problem is with *.a . They would load as the default >> > > behaviour of the dlopen. It is only when the dlopen fails for *.so , we >> > > give another chance to check for .a file with the same name. >> > >> > >> > No, what I meant, and what must be clarified before going forward with >> > this solution, is the following: >> > >> > * is _every_ `*.a` object on AIX loadable with `dlopen`, and will the >> > result be the same as when loading a `*.so` object >> > * or, if we present arbitrary `*.a` files to dlopen, is there a chance for >> > dlopen to crash or misbehave. >> > >> > Reason is that I was under the impression that *.a libraries are static >> > libraries and cannot be loaded dynamically. This is what you now try to do. >> > If we cannot safely answer this question, I would opt for a more narrow >> > solution by hard-wiring known alternative names. So, do the second *.a >> > attempt only for your `ibm_16_am.a` which you know works. That could also >> > be done in a reasonably maintainable manner. >> >> In AIX, both static and dynamic libraries have *.a extension. And AIX also >> supports *.so files.Bascially shared objects in AIX have both *.a and *.so >> extension. Hence we need to implement this logic. If we try loading a static >> archive specifically ,how the dlopen would behave , that is something >> probably @JoKern65 can answer ? > > Rather, this is a question you have to ask your collegues at IBM that develop > the AIX libc. > > Since AIX libc is not open source, we cannot look for ourselves, nor can > Joachim (her works at SAP). > >> >> > > > Does this really have to be handled in the OpenJDK? What does J9 on >> > > > AIX do? Could this be done in a simpler way outside OpenJDK, e.g. by >> > > > providing an *.so variant of the library in question? Where does this >> > > > library come from? >> > >> > >> > > I am not sure how J9 handles this. I would have to consult . >> > >> > >> > J9 is Open Source, can't you just look? :) >> >> I did try comparing the file structures, and i do not see a
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v7]
On Tue, 16 Jan 2024 16:12:16 GMT, Martin Doerr wrote: > > I have tried to build jextract > > (https://github.com/openjdk/jextract/tree/jdk22) with LLVM > > (https://github.com/llvm/llvm-project/releases/download/llvmorg-16.0.4/clang+llvm-16.0.4-powerpc64-ibm-aix-7.2.tar.xz). > > I noticed that llvm mainly consists of .a files. So, I think we need to > > support that for FFI compatibility with other libraries and open source > > projects. > > Seems like this change is not sufficient for that. `clang` is compiled to > `libclang.a` on AIX, but `libclang.so` on linux. I'm getting "System error: > Exec format error" when trying to load `libclang.a` via > `System.loadLibrary(libName);`. So the question remains: Are .a files really > supposed to be dynamically loadable on AIX? If so, where is that documented? Yes we have rhe case where .a files are dynamically loadable as well on AIX. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1905321150
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
On Mon, 22 Jan 2024 21:26:41 GMT, Volker Simonis wrote:
>> Currently we don't record dependencies on redefined methods (i.e.
>> `evol_method` dependencies) in JIT compiled methods if none of the
>> `can_redefine_classes`, `can_retransform_classes` or
>> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
>> if a JVMTI agent which requests one of these capabilities is dynamically
>> attached, all the methods which have been JIT compiled until that point,
>> will be marked for deoptimization and flushed from the code cache. For
>> large, warmed-up applications this mean deoptimization and instant
>> recompilation of thousands if not then-thousands of methods, which can lead
>> to dramatic performance/latency drop-downs for several minutes.
>>
>> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
>> 451: Prepare to Disallow the Dynamic Loading of
>> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
>> making the recording of `evol_method` dependencies dependent on the new
>> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
>> capabilities (because the presence of the flag indicates that an agent will
>> be loaded eventually).
>>
>> But there a single, however important exception to this rule and that's JFR.
>> JFR is advertised as low overhead profiler which can be enabled in
>> production at any time. However, when JFR is started dynamically (e.g.
>> through JCMD or JMX) it will silently load a HotSpot internl JVMTI agent
>> which requests the `can_retransform_classes` and retransforms some classes.
>> This will inevitably trigger the deoptimization of all compiled methods as
>> described above.
>>
>> I'd therefor like to propose to *always* and unconditionally record
>> `evol_method` dependencies in JIT compiled code by exporting the relevant
>> properties right at startup in `init_globals()`:
>> ```c++
>> jint init_globals() {
>>management_init();
>>JvmtiExport::initialize_oop_storage();
>> +#if INCLUDE_JVMTI
>> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>> + JvmtiExport::set_all_dependencies_are_recorded(true);
>> +#endif
>>
>>
>> My measurements indicate that the overhead of doing so is minimal (around 1%
>> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
>> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
>> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
>> with C2) resulting in an aggregated nmethod size of around ~40bm.
>> Additionally recording `evol_method` dependencies only increases this size
>> be about 400kb
>
> Volker Simonis has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Guard the feature with a diagnostic option and update the comments in the
> code
src/hotspot/share/runtime/init.cpp line 121:
> 119: if (AlwaysRecordEvolDependencies) {
> 120: JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
> 121: JvmtiExport::set_all_dependencies_are_recorded(true);
I think the check for AlwaysRecordEvolDependencies needs to be moved into
set_can_hotswap_or_post_breakpoint and set_all_dependencies_are_recorded,
otherwise don't we risk the value being accidentally reset to false when
set_can_hotswap_or_post_breakpoint() is called again by
JvmtiManageCapabilities::update()?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1462685351
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v2]
> 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed sendaoYan has updated the pull request incrementally with one additional commit since the last revision: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Signed-off-by: sendaoYan - Changes: - all: https://git.openjdk.org/jdk/pull/17514/files - new: https://git.openjdk.org/jdk/pull/17514/files/be81665d..969b608d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17514=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17514=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17514.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17514/head:pull/17514 PR: https://git.openjdk.org/jdk/pull/17514
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Mon, 22 Jan 2024 15:03:18 GMT, Severin Gehwolf wrote: > `1k` increments for a total of `512k` times seems overkill. Are you sure > that's needed to make the test pass? How about `1MB` increments for a total > of `512` times? When the docker serivice work normally on the test machine, this test will always fail. This test want to verify the API `jdk.internal.platform.Metrics.systemMetrics().getMemoryFailCount()` work normally or not. The API return memory allocate fail times in jvm. But, before this PR, everytime it allocate `1M` memory, the API has no chance the catch the memory allocate fail, the jvm was killed by OOM. Change `8M` increments to `1K` mean to avoid OOM killed for the jvm in docker container. jvm was killed by OOM in docker container:  `1M` Increnents also can avoid OOM killed.  - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-1905139487
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
On Mon, 22 Jan 2024 23:37:35 GMT, John R Rose wrote: >> src/hotspot/share/runtime/globals.hpp line 2013: >> >>> 2011: "Profile exception handlers") >>> \ >>> 2012: >>> \ >>> 2013: product(bool, AlwaysRecordEvolDependencies, true, DIAGNOSTIC, >>> \ >> >> As we record all dependencies not only evol_method ones, should we name it >> just: `AlwaysRecordDependencies`? > > That’s not exactly right either. `RecordAllDependencies` would be more like > it. Because: > > - We might record some dependencies that we know we need, and leave others > out. > - Or, we might record all dependencies, even ones we think we might not > need. > > (But we will need them all if somebody turns on JFR.) > > I like this change. Having a diagnostic switch means we can do a rough > triage if something seems to go wrong with this change, down the road. No, because you want to turn on/off evol_method independently of the other dependencies that the compiler is recording. - PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1462556836
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
On Mon, 22 Jan 2024 23:26:08 GMT, Evgeny Astigeevich wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Guard the feature with a diagnostic option and update the comments in the >> code > > src/hotspot/share/runtime/globals.hpp line 2013: > >> 2011: "Profile exception handlers") >> \ >> 2012: >> \ >> 2013: product(bool, AlwaysRecordEvolDependencies, true, DIAGNOSTIC, >> \ > > As we record all dependencies not only evol_method ones, should we name it > just: `AlwaysRecordDependencies`? That’s not exactly right either. `RecordAllDependencies` would be more like it. Because: - We might record some dependencies that we know we need, and leave others out. - Or, we might record all dependencies, even ones we think we might not need. (But we will need them all if somebody turns on JFR.) I like this change. Having a diagnostic switch means we can do a rough triage if something seems to go wrong with this change, down the road. > src/hotspot/share/runtime/globals.hpp line 2014: > >> 2012: >> \ >> 2013: product(bool, AlwaysRecordEvolDependencies, true, DIAGNOSTIC, >> \ >> 2014: "Unconditionally record method dependencies on class " >> \ > > "... record compiled method dependencies ..."? (yes, “compiled methods” or even “nmethods”, or “methods in code cache”) - PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1462556769 PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1462557389
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
On Mon, 22 Jan 2024 21:26:41 GMT, Volker Simonis wrote:
>> Currently we don't record dependencies on redefined methods (i.e.
>> `evol_method` dependencies) in JIT compiled methods if none of the
>> `can_redefine_classes`, `can_retransform_classes` or
>> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
>> if a JVMTI agent which requests one of these capabilities is dynamically
>> attached, all the methods which have been JIT compiled until that point,
>> will be marked for deoptimization and flushed from the code cache. For
>> large, warmed-up applications this mean deoptimization and instant
>> recompilation of thousands if not then-thousands of methods, which can lead
>> to dramatic performance/latency drop-downs for several minutes.
>>
>> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
>> 451: Prepare to Disallow the Dynamic Loading of
>> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
>> making the recording of `evol_method` dependencies dependent on the new
>> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
>> capabilities (because the presence of the flag indicates that an agent will
>> be loaded eventually).
>>
>> But there a single, however important exception to this rule and that's JFR.
>> JFR is advertised as low overhead profiler which can be enabled in
>> production at any time. However, when JFR is started dynamically (e.g.
>> through JCMD or JMX) it will silently load a HotSpot internl JVMTI agent
>> which requests the `can_retransform_classes` and retransforms some classes.
>> This will inevitably trigger the deoptimization of all compiled methods as
>> described above.
>>
>> I'd therefor like to propose to *always* and unconditionally record
>> `evol_method` dependencies in JIT compiled code by exporting the relevant
>> properties right at startup in `init_globals()`:
>> ```c++
>> jint init_globals() {
>>management_init();
>>JvmtiExport::initialize_oop_storage();
>> +#if INCLUDE_JVMTI
>> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>> + JvmtiExport::set_all_dependencies_are_recorded(true);
>> +#endif
>>
>>
>> My measurements indicate that the overhead of doing so is minimal (around 1%
>> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
>> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
>> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
>> with C2) resulting in an aggregated nmethod size of around ~40bm.
>> Additionally recording `evol_method` dependencies only increases this size
>> be about 400kb
>
> Volker Simonis has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Guard the feature with a diagnostic option and update the comments in the
> code
This looks good.
-
Marked as reviewed by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17509#pullrequestreview-1837600406
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
On Mon, 22 Jan 2024 21:26:41 GMT, Volker Simonis wrote:
>> Currently we don't record dependencies on redefined methods (i.e.
>> `evol_method` dependencies) in JIT compiled methods if none of the
>> `can_redefine_classes`, `can_retransform_classes` or
>> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
>> if a JVMTI agent which requests one of these capabilities is dynamically
>> attached, all the methods which have been JIT compiled until that point,
>> will be marked for deoptimization and flushed from the code cache. For
>> large, warmed-up applications this mean deoptimization and instant
>> recompilation of thousands if not then-thousands of methods, which can lead
>> to dramatic performance/latency drop-downs for several minutes.
>>
>> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
>> 451: Prepare to Disallow the Dynamic Loading of
>> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
>> making the recording of `evol_method` dependencies dependent on the new
>> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
>> capabilities (because the presence of the flag indicates that an agent will
>> be loaded eventually).
>>
>> But there a single, however important exception to this rule and that's JFR.
>> JFR is advertised as low overhead profiler which can be enabled in
>> production at any time. However, when JFR is started dynamically (e.g.
>> through JCMD or JMX) it will silently load a HotSpot internl JVMTI agent
>> which requests the `can_retransform_classes` and retransforms some classes.
>> This will inevitably trigger the deoptimization of all compiled methods as
>> described above.
>>
>> I'd therefor like to propose to *always* and unconditionally record
>> `evol_method` dependencies in JIT compiled code by exporting the relevant
>> properties right at startup in `init_globals()`:
>> ```c++
>> jint init_globals() {
>>management_init();
>>JvmtiExport::initialize_oop_storage();
>> +#if INCLUDE_JVMTI
>> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>> + JvmtiExport::set_all_dependencies_are_recorded(true);
>> +#endif
>>
>>
>> My measurements indicate that the overhead of doing so is minimal (around 1%
>> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
>> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
>> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
>> with C2) resulting in an aggregated nmethod size of around ~40bm.
>> Additionally recording `evol_method` dependencies only increases this size
>> be about 400kb
>
> Volker Simonis has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Guard the feature with a diagnostic option and update the comments in the
> code
src/hotspot/share/runtime/globals.hpp line 2014:
> 2012:
> \
> 2013: product(bool, AlwaysRecordEvolDependencies, true, DIAGNOSTIC,
> \
> 2014: "Unconditionally record method dependencies on class "
> \
"... record compiled method dependencies ..."?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1462553968
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
On Mon, 22 Jan 2024 21:26:41 GMT, Volker Simonis wrote:
>> Currently we don't record dependencies on redefined methods (i.e.
>> `evol_method` dependencies) in JIT compiled methods if none of the
>> `can_redefine_classes`, `can_retransform_classes` or
>> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
>> if a JVMTI agent which requests one of these capabilities is dynamically
>> attached, all the methods which have been JIT compiled until that point,
>> will be marked for deoptimization and flushed from the code cache. For
>> large, warmed-up applications this mean deoptimization and instant
>> recompilation of thousands if not then-thousands of methods, which can lead
>> to dramatic performance/latency drop-downs for several minutes.
>>
>> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
>> 451: Prepare to Disallow the Dynamic Loading of
>> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
>> making the recording of `evol_method` dependencies dependent on the new
>> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
>> capabilities (because the presence of the flag indicates that an agent will
>> be loaded eventually).
>>
>> But there a single, however important exception to this rule and that's JFR.
>> JFR is advertised as low overhead profiler which can be enabled in
>> production at any time. However, when JFR is started dynamically (e.g.
>> through JCMD or JMX) it will silently load a HotSpot internl JVMTI agent
>> which requests the `can_retransform_classes` and retransforms some classes.
>> This will inevitably trigger the deoptimization of all compiled methods as
>> described above.
>>
>> I'd therefor like to propose to *always* and unconditionally record
>> `evol_method` dependencies in JIT compiled code by exporting the relevant
>> properties right at startup in `init_globals()`:
>> ```c++
>> jint init_globals() {
>>management_init();
>>JvmtiExport::initialize_oop_storage();
>> +#if INCLUDE_JVMTI
>> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>> + JvmtiExport::set_all_dependencies_are_recorded(true);
>> +#endif
>>
>>
>> My measurements indicate that the overhead of doing so is minimal (around 1%
>> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
>> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
>> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
>> with C2) resulting in an aggregated nmethod size of around ~40bm.
>> Additionally recording `evol_method` dependencies only increases this size
>> be about 400kb
>
> Volker Simonis has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Guard the feature with a diagnostic option and update the comments in the
> code
src/hotspot/share/runtime/globals.hpp line 2013:
> 2011: "Profile exception handlers")
> \
> 2012:
> \
> 2013: product(bool, AlwaysRecordEvolDependencies, true, DIAGNOSTIC,
> \
As we record all dependencies not only evol_method ones, should we name it
just: `AlwaysRecordDependencies`?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1462550445
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing
On Sat, 20 Jan 2024 19:48:07 GMT, Volker Simonis wrote:
> Currently we don't record dependencies on redefined methods (i.e.
> `evol_method` dependencies) in JIT compiled methods if none of the
> `can_redefine_classes`, `can_retransform_classes` or
> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
> if a JVMTI agent which requests one of these capabilities is dynamically
> attached, all the methods which have been JIT compiled until that point, will
> be marked for deoptimization and flushed from the code cache. For large,
> warmed-up applications this mean deoptimization and instant recompilation of
> thousands if not then-thousands of methods, which can lead to dramatic
> performance/latency drop-downs for several minutes.
>
> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
> 451: Prepare to Disallow the Dynamic Loading of
> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
> making the recording of `evol_method` dependencies dependent on the new
> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
> capabilities (because the presence of the flag indicates that an agent will
> be loaded eventually).
>
> But there a single, however important exception to this rule and that's JFR.
> JFR is advertised as low overhead profiler which can be enabled in production
> at any time. However, when JFR is started dynamically (e.g. through JCMD or
> JMX) it will silently load a HotSpot internl JVMTI agent which requests the
> `can_retransform_classes` and retransforms some classes. This will inevitably
> trigger the deoptimization of all compiled methods as described above.
>
> I'd therefor like to propose to *always* and unconditionally record
> `evol_method` dependencies in JIT compiled code by exporting the relevant
> properties right at startup in `init_globals()`:
> ```c++
> jint init_globals() {
>management_init();
>JvmtiExport::initialize_oop_storage();
> +#if INCLUDE_JVMTI
> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
> + JvmtiExport::set_all_dependencies_are_recorded(true);
> +#endif
>
>
> My measurements indicate that the overhead of doing so is minimal (around 1%
> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
> with C2) resulting in an aggregated nmethod size of around ~40bm.
> Additionally recording `evol_method` dependencies only increases this size be
> about 400kb. The ration remains about the same i...
Thanks everybody for looking at this. I've now guarded the feature with a
diagnostic command, updated the source code comments around
`VM_RedefineClasses::flush_dependent_code` and added an assertion for the new
flag.
-
PR Comment: https://git.openjdk.org/jdk/pull/17509#issuecomment-1904842722
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing [v2]
> Currently we don't record dependencies on redefined methods (i.e.
> `evol_method` dependencies) in JIT compiled methods if none of the
> `can_redefine_classes`, `can_retransform_classes` or
> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
> if a JVMTI agent which requests one of these capabilities is dynamically
> attached, all the methods which have been JIT compiled until that point, will
> be marked for deoptimization and flushed from the code cache. For large,
> warmed-up applications this mean deoptimization and instant recompilation of
> thousands if not then-thousands of methods, which can lead to dramatic
> performance/latency drop-downs for several minutes.
>
> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
> 451: Prepare to Disallow the Dynamic Loading of
> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
> making the recording of `evol_method` dependencies dependent on the new
> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
> capabilities (because the presence of the flag indicates that an agent will
> be loaded eventually).
>
> But there a single, however important exception to this rule and that's JFR.
> JFR is advertised as low overhead profiler which can be enabled in production
> at any time. However, when JFR is started dynamically (e.g. through JCMD or
> JMX) it will silently load a HotSpot internl JVMTI agent which requests the
> `can_retransform_classes` and retransforms some classes. This will inevitably
> trigger the deoptimization of all compiled methods as described above.
>
> I'd therefor like to propose to *always* and unconditionally record
> `evol_method` dependencies in JIT compiled code by exporting the relevant
> properties right at startup in `init_globals()`:
> ```c++
> jint init_globals() {
>management_init();
>JvmtiExport::initialize_oop_storage();
> +#if INCLUDE_JVMTI
> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
> + JvmtiExport::set_all_dependencies_are_recorded(true);
> +#endif
>
>
> My measurements indicate that the overhead of doing so is minimal (around 1%
> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
> with C2) resulting in an aggregated nmethod size of around ~40bm.
> Additionally recording `evol_method` dependencies only increases this size be
> about 400kb. The ration remains about the same i...
Volker Simonis has updated the pull request incrementally with one additional
commit since the last revision:
Guard the feature with a diagnostic option and update the comments in the code
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17509/files
- new: https://git.openjdk.org/jdk/pull/17509/files/95db4a72..6d3e24ab
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=17509=01
- incr: https://webrevs.openjdk.org/?repo=jdk=17509=00-01
Stats: 20 lines in 3 files changed: 8 ins; 0 del; 12 mod
Patch: https://git.openjdk.org/jdk/pull/17509.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17509/head:pull/17509
PR: https://git.openjdk.org/jdk/pull/17509
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing
On Sat, 20 Jan 2024 19:48:07 GMT, Volker Simonis wrote:
> Currently we don't record dependencies on redefined methods (i.e.
> `evol_method` dependencies) in JIT compiled methods if none of the
> `can_redefine_classes`, `can_retransform_classes` or
> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
> if a JVMTI agent which requests one of these capabilities is dynamically
> attached, all the methods which have been JIT compiled until that point, will
> be marked for deoptimization and flushed from the code cache. For large,
> warmed-up applications this mean deoptimization and instant recompilation of
> thousands if not then-thousands of methods, which can lead to dramatic
> performance/latency drop-downs for several minutes.
>
> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
> 451: Prepare to Disallow the Dynamic Loading of
> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
> making the recording of `evol_method` dependencies dependent on the new
> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
> capabilities (because the presence of the flag indicates that an agent will
> be loaded eventually).
>
> But there a single, however important exception to this rule and that's JFR.
> JFR is advertised as low overhead profiler which can be enabled in production
> at any time. However, when JFR is started dynamically (e.g. through JCMD or
> JMX) it will silently load a HotSpot internl JVMTI agent which requests the
> `can_retransform_classes` and retransforms some classes. This will inevitably
> trigger the deoptimization of all compiled methods as described above.
>
> I'd therefor like to propose to *always* and unconditionally record
> `evol_method` dependencies in JIT compiled code by exporting the relevant
> properties right at startup in `init_globals()`:
> ```c++
> jint init_globals() {
>management_init();
>JvmtiExport::initialize_oop_storage();
> +#if INCLUDE_JVMTI
> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
> + JvmtiExport::set_all_dependencies_are_recorded(true);
> +#endif
>
>
> My measurements indicate that the overhead of doing so is minimal (around 1%
> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
> with C2) resulting in an aggregated nmethod size of around ~40bm.
> Additionally recording `evol_method` dependencies only increases this size be
> about 400kb. The ration remains about the same i...
I support adding this as a diagnostic option, therefore you won't have to make
the change in VM_RedefineClasses::flush_dependent_code.
-
PR Comment: https://git.openjdk.org/jdk/pull/17509#issuecomment-1904784430
Re: RFR: 8324066: "clhsdb jstack" should not by default scan for j.u.c locks because it can be very slow [v2]
On Thu, 18 Jan 2024 16:19:38 GMT, Chris Plummer wrote: >> I noticed that "clhsdb jstack" seemed to hang when I attached to process >> with a somewhat large heap. It had taken over 10 minutes when I finally >> decided to have a look at the SA process (using bin/jstack), which came up >> with the following in the stack: >> >> >> at >> sun.jvm.hotspot.oops.ObjectHeap.iterateObjectsOfKlass([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/ObjectHeap.java:117) >> at >> sun.jvm.hotspot.runtime.ConcurrentLocksPrinter.fillLocks([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/ConcurrentLocksPrinter.java:70) >> at >> sun.jvm.hotspot.runtime.ConcurrentLocksPrinter.([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/ConcurrentLocksPrinter.java:36) >> at >> sun.jvm.hotspot.tools.StackTrace.run([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/StackTrace.java:71) >> >> >> This code is meant to print information about j.u.c. locks. It it works by >> searching the entire java heap for instances of AbstractOwnableSynchronizer. >> This is a very expensive operation because it means not only does SA need to >> read in the entire java heap, but it needs to create a Klass mirror for >> every heap object so it can determine its type. >> >> Our testing doesn't seem to run into this slowness problem because "clhsdb >> jstack" only iterates over the heap if AbstractOwnableSynchronizer has been >> loaded, which it won't be if no j.u.c. locks have been created. This seems >> to be the case wherever we use "clhsdb jstack" in testing. We do have some >> tests that likely result in j.u.c locks, but they all use "jhsdb jstack", >> which doesn't have this issue (it requires use of the --locks argument to >> enable printing of j.u.c locks). >> >> This issue was already called out in >> [JDK-8262098](https://bugs.openjdk.org/browse/JDK-8262098). For this CR I am >> proposing that "clhsdb jstack" not include j.u.c lock scanning by default, >> and add the -l argument to enable it. This will make it similar to >> bin/jstack, which also has a -l argument, and to "clhsdb jstack", which has >> a --locks argument (which maps internally to the Jstack.java -l argument). >> >> The same changes are also being made to "clhsdb pstack". >> >> Tested with all tier1, tier2, and tier5 svc tests. > > Chris Plummer has updated the pull request incrementally with two additional > commits since the last revision: > > - update clhsdb docs > - update clhsdb docs Ping! I still need 2 reviews. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/17479#issuecomment-1904625954
Re: RFR: 8323680: SA PointerFinder code can do a better job of leveraging existing code to determine if an address is in the TLAB
On Thu, 18 Jan 2024 22:54:26 GMT, Chris Plummer wrote:
> In PointerFinder.java we have some code to determine if a pointer is in a
> TLAB, but it only executes for the SerialGC. It should work for all GCs, so I
> moved the code out of the SerialGC block.
>
> I also cleaned up the printing in PointerLocation. java a bit so when not
> using verbose mode not as much info about the tlab address is printed. This
> is consistent with other addresses, such as java stack addresses, which is
> what I modeled this change on.
>
> It's hard to test this change since it is hard to consistently get an address
> to be in the tlab. I wrote a little test program that just sits in a loop
> doing allocations. I attached to it with clhsdb and ran the threadcontext
> command, which does a fincpc on each register. About half the time the main
> thread was suspended in a frame where some registers where pointing into the
> tlab, and I confirmed this was the case for both SerialGC and G1. Here's an
> example of one register with verbose off and verbose on:
>
> rsi: 0x8a5d4448: In TLAB for thread "main"
> sun.jvm.hotspot.runtime.JavaThread@0x7ffa24029000
>
> rsi: 0x8a5d4448: In TLAB for thread ("main" #1 prio=5
> tid=0x7ffa24029000 nid=25392 runnable [0x]
>java.lang.Thread.State: RUNNABLE
>JavaThread state: _thread_in_java
> )
> [0x8a5d4448,0x8ab724b8,0x8b0c0250,{0x8b0c0490})
>
> For testing I ran all tier1, tier2, and tier5 svc tests (still in progress)
Ping! I still need 2 reviews. Thanks!
-
PR Comment: https://git.openjdk.org/jdk/pull/17494#issuecomment-1904625532
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing
On Mon, 22 Jan 2024 16:37:14 GMT, Aleksey Shipilev wrote:
>> Currently we don't record dependencies on redefined methods (i.e.
>> `evol_method` dependencies) in JIT compiled methods if none of the
>> `can_redefine_classes`, `can_retransform_classes` or
>> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
>> if a JVMTI agent which requests one of these capabilities is dynamically
>> attached, all the methods which have been JIT compiled until that point,
>> will be marked for deoptimization and flushed from the code cache. For
>> large, warmed-up applications this mean deoptimization and instant
>> recompilation of thousands if not then-thousands of methods, which can lead
>> to dramatic performance/latency drop-downs for several minutes.
>>
>> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
>> 451: Prepare to Disallow the Dynamic Loading of
>> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
>> making the recording of `evol_method` dependencies dependent on the new
>> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
>> capabilities (because the presence of the flag indicates that an agent will
>> be loaded eventually).
>>
>> But there a single, however important exception to this rule and that's JFR.
>> JFR is advertised as low overhead profiler which can be enabled in
>> production at any time. However, when JFR is started dynamically (e.g.
>> through JCMD or JMX) it will silently load a HotSpot internl JVMTI agent
>> which requests the `can_retransform_classes` and retransforms some classes.
>> This will inevitably trigger the deoptimization of all compiled methods as
>> described above.
>>
>> I'd therefor like to propose to *always* and unconditionally record
>> `evol_method` dependencies in JIT compiled code by exporting the relevant
>> properties right at startup in `init_globals()`:
>> ```c++
>> jint init_globals() {
>>management_init();
>>JvmtiExport::initialize_oop_storage();
>> +#if INCLUDE_JVMTI
>> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>> + JvmtiExport::set_all_dependencies_are_recorded(true);
>> +#endif
>>
>>
>> My measurements indicate that the overhead of doing so is minimal (around 1%
>> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
>> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
>> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
>> with C2) resulting in an aggregated nmethod size of around ~40bm.
>> Additionally recording `evol_method` dependencies only increases this size
>> be about 400kb
>
>> Instead of unconditionally recording evol_method dependencies we could guard
>> the recording by a new flag. But this would only make sense if that flag
>> would be on by default and I don't know if such a flag is justified just for
>> the rare (or non-existent?) cases where somebody wants to disable the
>> recording of the dependencies.
>
> I think introducing a diagnostic flag is sensible here. If we figure out much
> later that this solution comes with some other (worse) problems, the
> diagnostic flag gives us the options to: a) clearly point at this addition as
> the culprit; b) have the easily deployable solution to restore the original
> behavior.
>
> For the change itself, we need to amend the comment near
> `VM_RedefineClasses::flush_dependent_code` definition that talks about this
> peculiar behavior, which now changes. Actually, maybe even the implementation
> of `flush_dependent_code` should now trust (and assert) that all dependencies
> are now recorded?
@shipilev I filed https://bugs.openjdk.org/browse/JDK-8324318 which is related
to this change.
-
PR Comment: https://git.openjdk.org/jdk/pull/17509#issuecomment-1904402169
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing
On Sat, 20 Jan 2024 19:48:07 GMT, Volker Simonis wrote:
> Currently we don't record dependencies on redefined methods (i.e.
> `evol_method` dependencies) in JIT compiled methods if none of the
> `can_redefine_classes`, `can_retransform_classes` or
> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
> if a JVMTI agent which requests one of these capabilities is dynamically
> attached, all the methods which have been JIT compiled until that point, will
> be marked for deoptimization and flushed from the code cache. For large,
> warmed-up applications this mean deoptimization and instant recompilation of
> thousands if not then-thousands of methods, which can lead to dramatic
> performance/latency drop-downs for several minutes.
>
> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
> 451: Prepare to Disallow the Dynamic Loading of
> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
> making the recording of `evol_method` dependencies dependent on the new
> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
> capabilities (because the presence of the flag indicates that an agent will
> be loaded eventually).
>
> But there a single, however important exception to this rule and that's JFR.
> JFR is advertised as low overhead profiler which can be enabled in production
> at any time. However, when JFR is started dynamically (e.g. through JCMD or
> JMX) it will silently load a HotSpot internl JVMTI agent which requests the
> `can_retransform_classes` and retransforms some classes. This will inevitably
> trigger the deoptimization of all compiled methods as described above.
>
> I'd therefor like to propose to *always* and unconditionally record
> `evol_method` dependencies in JIT compiled code by exporting the relevant
> properties right at startup in `init_globals()`:
> ```c++
> jint init_globals() {
>management_init();
>JvmtiExport::initialize_oop_storage();
> +#if INCLUDE_JVMTI
> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
> + JvmtiExport::set_all_dependencies_are_recorded(true);
> +#endif
>
>
> My measurements indicate that the overhead of doing so is minimal (around 1%
> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
> with C2) resulting in an aggregated nmethod size of around ~40bm.
> Additionally recording `evol_method` dependencies only increases this size be
> about 400kb. The ration remains about the same i...
Marked as reviewed by phh (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/17509#pullrequestreview-1836891644
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing
On Sat, 20 Jan 2024 19:48:07 GMT, Volker Simonis wrote: > Instead of unconditionally recording evol_method dependencies we could guard > the recording by a new flag. But this would only make sense if that flag > would be on by default and I don't know if such a flag is justified just for > the rare (or non-existent?) cases where somebody wants to disable the > recording of the dependencies. I think introducing a diagnostic flag is sensible here. If we figure out much later that this solution comes with some other (worse) problems, the diagnostic flag gives us the options to: a) clearly point at this addition as the culprit; b) have the easily deployable solution to restore the original behavior. For the change itself, we need to amend the comment near `VM_RedefineClasses::flush_dependent_code` definition that talks about this peculiar behavior, which now changes. Actually, maybe even the implementation of `flush_dependent_code` should now trust (and assert) that all dependencies are now recorded? - PR Comment: https://git.openjdk.org/jdk/pull/17509#issuecomment-1904382556
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Mon, 22 Jan 2024 09:31:43 GMT, sendaoYan wrote: > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed `1k` increments for a total of `512k` times seems overkill. Are you sure that's needed to make the test pass? How about `1MB` increments for a total of `512` times? - PR Review: https://git.openjdk.org/jdk/pull/17514#pullrequestreview-1836685784
Re: RFR: 8324241: Always record evol_method deps to avoid excessive method flushing
On Sat, 20 Jan 2024 19:48:07 GMT, Volker Simonis wrote:
> Currently we don't record dependencies on redefined methods (i.e.
> `evol_method` dependencies) in JIT compiled methods if none of the
> `can_redefine_classes`, `can_retransform_classes` or
> `can_generate_breakpoint_events` JVMTI capabalities is set. This means that
> if a JVMTI agent which requests one of these capabilities is dynamically
> attached, all the methods which have been JIT compiled until that point, will
> be marked for deoptimization and flushed from the code cache. For large,
> warmed-up applications this mean deoptimization and instant recompilation of
> thousands if not then-thousands of methods, which can lead to dramatic
> performance/latency drop-downs for several minutes.
>
> One could argue that dynamic agent attach is now deprecated anyway (see [JEP
> 451: Prepare to Disallow the Dynamic Loading of
> Agents](https://openjdk.org/jeps/451)) and this problem could be solved by
> making the recording of `evol_method` dependencies dependent on the new
> `-XX:+EnableDynamicAgentLoading` flag isntead of the concrete JVMTI
> capabilities (because the presence of the flag indicates that an agent will
> be loaded eventually).
>
> But there a single, however important exception to this rule and that's JFR.
> JFR is advertised as low overhead profiler which can be enabled in production
> at any time. However, when JFR is started dynamically (e.g. through JCMD or
> JMX) it will silently load a HotSpot internl JVMTI agent which requests the
> `can_retransform_classes` and retransforms some classes. This will inevitably
> trigger the deoptimization of all compiled methods as described above.
>
> I'd therefor like to propose to *always* and unconditionally record
> `evol_method` dependencies in JIT compiled code by exporting the relevant
> properties right at startup in `init_globals()`:
> ```c++
> jint init_globals() {
>management_init();
>JvmtiExport::initialize_oop_storage();
> +#if INCLUDE_JVMTI
> + JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
> + JvmtiExport::set_all_dependencies_are_recorded(true);
> +#endif
>
>
> My measurements indicate that the overhead of doing so is minimal (around 1%
> increase of nmethod size) and justifies the benefit. E.g. a Spring Petclinic
> application started with `-Xbatch -XX:+UnlockDiagnosticVMOptions
> -XX:+LogCompilation` compiles about ~11500 methods (~9000 with C1 and ~2500
> with C2) resulting in an aggregated nmethod size of around ~40bm.
> Additionally recording `evol_method` dependencies only increases this size be
> about 400kb. The ration remains about the same i...
lgtm
-
Marked as reviewed by eastigeevich (Committer).
PR Review: https://git.openjdk.org/jdk/pull/17509#pullrequestreview-1836432196
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Mon, 22 Jan 2024 09:31:43 GMT, sendaoYan wrote: > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed The test case before this PR has a maximum heap of 64MB and applies for 8M of memory each time in the for loop. When applying for memory for the sixth time, it was killed by the docker container because of OOM, jdk.internal.platform.Metrics.systemMetrics().getMemoryFailCount( ) interface has no chance to return 1, and the Java process returns exit code 137. The maximum heap is also 64M, The PR is changed to 1KB each time to ensure that the getMemoryFailCount() interface has a chance to return 1 and the test case has a chance to exit the for loop of memory allocation. ## test result before this PR:  ## test result after this PR:  - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-1903589872
RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed - Commit messages: - 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Changes: https://git.openjdk.org/jdk/pull/17514/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17514=00 Issue: https://bugs.openjdk.org/browse/JDK-8323640 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17514.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17514/head:pull/17514 PR: https://git.openjdk.org/jdk/pull/17514
Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]
On Mon, 22 Jan 2024 08:59:30 GMT, Kim Barrett wrote: > I don't think `#pragma GCC poison` works for us. It would complain about a > system or library header that uses NULL and is included after the pragma. I see, that's a shame in that case > However, there are still a lot of NULL's left. All of the per-cpu .ad files, > and the jvmtiXXX.xsl files contain NULL's that will appear in the associated > generated code. Also, NULL usage in gtests doesn't seem to have been > addressed yet. > > But it does look like there's been a bit of backsliding: > https://bugs.openjdk.org/browse/JDK-8324286 That's a little worrying, maybe the Style Guide's NULL section could be reworded since now most usages of NULL are nullptr? The .ad files and .xsl files are a bit of a problem though > MSVC's deprecation pragma might work for this, at least for shared and > Windows-specific code. I couldn't find a way to use it for FORBID_C_FUNCTION, > but the problems I encountered for that don't seem applicable in this case. The deprecation pragma only works for macros and identifiers, it doesn't accept method signatures and would warn for every time a identifier is used, even in the method's declaration itself! Probably can't be used in FORBID_C_FUNCTION as mentioned above, but sounds good for a macro like NULL I've also been trying to implement FORBID_C_FUNCTION and ALLOW_C_FUNCTION portably, speaking of it, but it hasn't been going great so far :/ https://github.com/openjdk/jdk/pull/17387 - PR Comment: https://git.openjdk.org/jdk/pull/14198#issuecomment-1903579473
Withdrawn: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
On Fri, 12 Jan 2024 03:31:37 GMT, sendaoYan wrote: > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/17386
Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]
On Tue, 30 May 2023 19:15:38 GMT, Johan Sjölen wrote: >> A final sweep of Hotspot to remove all re-added NULLs. With only 110 changes >> I'd appreciate if this was considered trivial. > > Johan Sjölen has updated the pull request incrementally with two additional > commits since the last revision: > > - Align > - Suggestions I don't think `#pragma GCC poison` works for us. It would complain about a system or library header that uses NULL and is included after the pragma. MSVC's deprecation pragma might work for this, at least for shared and Windows-specific code. I couldn't find a way to use it for FORBID_C_FUNCTION, but the problems I encountered for that don't seem applicable in this case. However, there are still a lot of NULL's left. All of the per-cpu .ad files, and the jvmtiXXX.xsl files contain NULL's that will appear in the associated generated code. Also, NULL usage in gtests doesn't seem to have been addressed yet. But it does look like there's been a bit of backsliding: https://bugs.openjdk.org/browse/JDK-8324286 - PR Comment: https://git.openjdk.org/jdk/pull/14198#issuecomment-1903531104
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v2]
> 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed sendaoYan has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Signed-off-by: sendaoYan - Changes: - all: https://git.openjdk.org/jdk/pull/17386/files - new: https://git.openjdk.org/jdk/pull/17386/files/e8a99fe4..9f0aa2a1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17386=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17386=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17386.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17386/head:pull/17386 PR: https://git.openjdk.org/jdk/pull/17386
