RFR: 8337217: Port VirtualMemoryTracker to use VMATree
Afshin Zafari
azafari at openjdk.org
Fri Nov 8 10:51:42 UTC 2024
On Thu, 26 Sep 2024 10:58:31 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> What is the consensus on having two implementations alive at the same time? I'd like to see us delete the old VirtualMemoryTracker and only have the tree implementation left as part of this PR. What is the status of our testing? Is the tree version equivalent now and passes all tests?
>
I have already told that the main reason to have 2 versions is to be able to switch back from new to old version at customer site at runtime. This is what Thomas has requested.
During the implementation, it is much beneficial to have access to the old version to compare the results and debug problems when they occur.
Until the whole port to VMATree is not done, we need this 2 versions side-by-side feature.
> That's good, let's wait until that PR is integrated before this is integrated.
Sure, I do.
> Is this part of the code under-tested as we didn't receive a test failure regarding this?
All JTREG runtime/NMT tests and all tier1 tests have been passed on this code.
> Note that we can't do the insertion in the iteration loop, as our iterator will be invalidated.
I meant that the iterator is not going through the nodes in VMATree. And the `add_committeed_region` is called on a local `ReservedMemoryRegion*`.
Anyway, I agree to rewrite this for many more reasons than above, but not sure how. The required change was deferred for a later time when we started new VMT. Should think about it.
> src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp line 262:
>
>> 260: vm_exit_out_of_memory(word_size * BytesPerWord, OOM_MMAP_ERROR, "Failed to reserve memory for metaspace");
>> 261: }
>> 262: assert(rs.size() == word_size * BytesPerWord, " must be");
>
> " must be" -> "must be"
Done
> src/hotspot/share/nmt/memReporter.cpp line 442:
>
>> 440: if (all_committed) {
>> 441: bool reserved_and_committed = false;
>> 442: VirtualMemoryTracker::Instance::tree()->visit_committed_regions(*reserved_rgn,
>
> Change the signature of `visit_committed_regions` to taking `(position start, size size)` instead of the `ReservedMemoryRegion`.
Done.
> src/hotspot/share/nmt/memReporter.cpp line 458:
>
>> 456: }
>> 457:
>> 458: auto print_committed_rgn = [&](CommittedMemoryRegion* crgn) {
>
> `crgn` can be const reference instead of pointer.
Done.
> src/hotspot/share/nmt/memTracker.hpp line 291:
>
>> 289: // Query lock
>> 290: static Mutex* _query_lock;
>> 291:
>
> Remove
Done.
> src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp line 93:
>
>> 91:
>> 92: const inline NativeCallStack& get(StackIndex si) {
>> 93: if (is_invalid(si) || si >= _stacks.length()) {
>
> I don't think this should be here?
Removed.
> src/hotspot/share/nmt/regionsTree.cpp line 33:
>
>> 31: log_debug(nmt)("trc base: " INTPTR_FORMAT " , trc end: " INTPTR_FORMAT,
>> 32: p2i(region_in_tree.base()), p2i(region_in_tree.end()));
>> 33: }
>
> Remove `with_trace` and improve the logging message. This should log unconditionally, probably under a `(nmt,vmtracker)` tagset.
Done.
> src/hotspot/share/nmt/regionsTree.cpp line 50:
>
>> 48: ReservedMemoryRegion rgn = find_reserved_region(addr);
>> 49: return reserve_mapping((VMATree::position)addr, size, make_region_data(NativeCallStack::empty_stack(), rgn.mem_tag()));
>> 50: }
>
> Can now be fixed.
Done.
> src/hotspot/share/nmt/regionsTree.cpp line 52:
>
>> 50: rgn = find_reserved_region(addr, true);
>> 51: ShouldNotReachHere();
>> 52: }
>
> TODO: Remove this when 8335091 is merged
Added a TODO comment to remind it.
> src/hotspot/share/nmt/regionsTree.cpp line 54:
>
>> 52: }
>> 53: return commit_mapping((VMATree::position)addr, size, make_region_data(stack, rgn.flag()));
>> 54:
>
> Style: Extra space
Fixed.
> src/hotspot/share/nmt/regionsTree.cpp line 64:
>
>> 62: rgn = find_reserved_region(addr, true);
>> 63: ShouldNotReachHere();
>> 64: }
>
> TODO: Remove this when https://github.com/openjdk/jdk/commit/8335091a5dd9a0654d9a616addcfdcc1863747ca is merged
Added a TODO comment to remind this.
> src/hotspot/share/nmt/regionsTree.hpp line 31:
>
>> 29: #include "nmt/vmatree.hpp"
>> 30:
>> 31: class RegionsTree : public VMATree {
>
> Is it necessary for this to be a superclass? Seems like this class contains utilities for working with a `VMATree`. Could it instead be `AllStatic` and take a tree as its first argument? I'd like to avoid inheritance unless necessary (typically for the usage of virtual functions).
It is inherited because it is not a new type. I think that `_tree->visit_reserved_region(par1, par2)` is more readable than `VMATree::visit_reserved_region(_tree, par1, par2)`
I could not find any *virtual functions* in these classes, what do you mean by that?
> src/hotspot/share/nmt/regionsTree.hpp line 33:
>
>> 31: class RegionsTree : public VMATree {
>> 32: private:
>> 33: NativeCallStackStorage* _ncs_storage;
>
> No need for this to be a pointer.
How to use the ctor with a `bool` arg, then?
> src/hotspot/share/nmt/regionsTree.hpp line 38:
>
>> 36: public:
>> 37: RegionsTree(bool with_storage) : VMATree() , _ncs_storage(with_storage) { }
>> 38:
>
> Whitespace
Done.
> src/hotspot/share/nmt/regionsTree.hpp line 47:
>
>> 45:
>> 46: class NodeHelper {
>> 47: Node *_node;
>
> Style: `*` should hug type, not name.
Fixed.
> src/hotspot/share/nmt/regionsTree.hpp line 50:
>
>> 48: inline bool is_before(VMATree::position addr) { return VMATree::PositionComparator::cmp(key(), addr) < 0; }
>> 49: inline bool is_after(VMATree::position addr) { return VMATree::PositionComparator::cmp(key(), addr) > 0; }
>> 50: inline bool is_region_begin() { return is_reserved_begin()|| is_committed_begin(); }
>
> Not used?
Removed non-used methods.
> src/hotspot/share/nmt/regionsTree.hpp line 52:
>
>> 50: inline bool is_region_begin() { return is_reserved_begin()|| is_committed_begin(); }
>> 51: inline bool is_region_end(NodeHelper* node) { return is_released_begin() || (node != nullptr && node->out_flag() != out_flag()); }
>> 52: inline bool is_end_only(NodeHelper* node) { return !is_region_begin() && is_region_end(node); }
>
> Only used for debugging?
Done.
> src/hotspot/share/nmt/regionsTree.hpp line 53:
>
>> 51: inline bool is_region_end(NodeHelper* node) { return is_released_begin() || (node != nullptr && node->out_flag() != out_flag()); }
>> 52: inline bool is_end_only(NodeHelper* node) { return !is_region_begin() && is_region_end(node); }
>> 53: inline bool is_joint(NodeHelper* node) { return is_region_begin() && is_region_end(node); }
>
> Only used for debugging?
Not anymore. Removed.
> src/hotspot/share/nmt/regionsTree.hpp line 58:
>
>> 56: // inline bool is_region_end(NodeHelper* node) { return is_released_begin() || (node != nullptr && node->out_flag() != out_flag()); }
>> 57: // inline bool is_end_only(NodeHelper* node) { return !is_region_begin() && is_region_end(node); }
>> 58: // inline bool is_joint(NodeHelper* node) { return is_region_begin() && is_region_end(node); }
>
> No commented out code like this please.
OK. Fixed.
> src/hotspot/share/nmt/regionsTree.hpp line 61:
>
>> 59: inline VMATree::StateType out_state() { return val().out.type(); }
>> 60: inline size_t distance_from(NodeHelper* other) { return key() - other->key(); }
>> 61: inline const NativeCallStack& stack(NativeCallStackStorage* ncss) { return ncss->get(val().in.stack()); }
>
> Remove this
Changed to return StackIndex.
> src/hotspot/share/nmt/regionsTree.hpp line 66:
>
>> 64: inline VMATree::StateType out_state() { return _node->val().out.type(); }
>> 65: inline size_t distance_from(NodeHelper& other) { return position() - other.position(); }
>> 66: inline const NativeCallStackStorage::StackIndex out_stack_index() { return _node->val().out.stack(); }
>
> Unnecessary const in return type.
Fixed.
> src/hotspot/share/nmt/regionsTree.hpp line 71:
>
>> 69: inline void set_in_flag(MEMFLAGS flag) { _node->val().in.set_flag(flag); }
>> 70: inline void set_out_flag(MEMFLAGS flag) { _node->val().out.set_flag(flag); }
>> 71: inline void dump(outputStream* st) {
>
> `dump` is typically called `print_on` in HotSpot.
`print_on` is used.
> src/hotspot/share/nmt/regionsTree.hpp line 91:
>
>> 89:
>> 90: template<typename F> // F == bool(*)(CommittedMemoryRegion*)
>> 91: void visit_committed_regions(const ReservedMemoryRegion* rgn, CommittedMemoryRegion* cmr, F func) {
>
> Why does it take the `ReservedMemoryRegion` as a pointer instead of a reference? There's no need to take a pointer to the `CommittedMemoryRegion`, just construct it in the function and pass it to `f`.
Fixed.
> src/hotspot/share/nmt/regionsTree.hpp line 91:
>
>> 89: template<typename F> // F == bool(*)(CommittedMemoryRegion&)
>> 90: void visit_committed_regions(position start, size_t size, F func) {
>> 91: size_t end = start + size + 1;
>
> It's not clear to me why `+ 1` is needed. If we have a range `[start, size+start)`, will it be missed by `visit_range_in_order`?
Yes it is missed.
In the `visit_range_in_order` code, the variable `cmp_to` is never checked against `0` (`cmp_to == 0`).
> src/hotspot/share/nmt/regionsTree.hpp line 95:
>
>> 93: size_t end = (size_t)rgn->end();
>> 94: size_t comm_size = 0;
>> 95: size_t base = (size_t)rgn->base();
>
> Why casting these?
Because the args to `visit_range_in_order` are of type `position` which is `size_t`. Otherwise, compiler complains about conversion from `address` to `size_t`.
> src/hotspot/share/nmt/regionsTree.hpp line 108:
>
>> 106: CommittedMemoryRegion cmr((address)base, comm_size, st);
>> 107: comm_size = 0;
>> 108: prev.clear_node();
>
> This is unneeded.
Which part? Why?
`clear_node()` is the same as `prev = nullptr` if pointers were used.
`is_valid()` is the same as `prev == nullptr` if pointers were used.
> src/hotspot/share/nmt/regionsTree.hpp line 140:
>
>> 138: if (r_size != rgn_size) {
>> 139: log_debug(nmt)("----------------- size differ, distance: " SIZE_FORMAT " size: " SIZE_FORMAT, r_size, rgn_size);
>> 140: }
>
> Not needed anymore?
Removed.
> src/hotspot/share/nmt/regionsTree.hpp line 143:
>
>> 141: visit_in_order([&](Node* node) {
>> 142: NodeHelper* curr = (NodeHelper*)node;
>> 143: /*
>
> Delete
Done.
> src/hotspot/share/nmt/regionsTree.hpp line 172:
>
>> 170: inline const NativeCallStack stack(NodeHelper& node) {
>> 171: NativeCallStackStorage::StackIndex si = node.out_stack_index();
>> 172: if (!si.is_invalid()) {
>
> Change check to `NativeCallStackStorage::is_invalid`.
Done.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.cpp line 101:
>
>> 99: }
>> 100:
>> 101: void VirtualMemoryTrackerWithTree::apply_summary_diff(VMATree::SummaryDiff diff) {
>
> Applying a summary diff in the MemoryFileTracker is this:
>
> ```c++
> for (int i = 0; i < mt_number_of_types; i++) {
> VirtualMemory* summary = file->_summary.by_type(NMTUtil::index_to_flag(i));
> summary->reserve_memory(diff.flag[i].commit);
> summary->commit_memory(diff.flag[i].commit);
> }
>
>
> This is much simpler and doesn't require looking at signs and so on.
In `MemoryFileTracker`, the changes in commit/reserve are applied to a local `VirtualMemorySnapshot`. Here we have to apply them to the global `VirtualMemorySummary`.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.cpp line 109:
>
>> 107: return true;
>> 108: });
>> 109: }
>
> TODO: This should be replaced with the implementation in 8340103
>
> Will happen when that PR has been reviewed and integrated.
Replaced.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.cpp line 115:
>
>> 113: " vms-committed: " SIZE_FORMAT,
>> 114: str, NMTUtil::flag_to_name(flag), r, c, reserved, committed);
>> 115: };
>
> Move this up to before the loop.
Done.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 34:
>
>> 32: #include "utilities/ostream.hpp"
>> 33:
>> 34: class VirtualMemoryTrackerWithTree : AllStatic {
>
> Can this class be redone to use the same pattern as `MemoryFileTracker` with an outer class that's not static and an inner class that's stack, storing an instance of the outer class? That makes testing the class by creating an instance of it possible, unlike our old tests.
Can we do this change as a separate RFE? It impacts the code a lot.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 41:
>
>> 39: VirtualMemoryTrackerWithTree(bool is_detailed_mode) : _tree(is_detailed_mode) { }
>> 40:
>> 41: bool add_reserved_region (address base_addr, size_t size, const NativeCallStack& stack, MEMFLAGS flag = mtNone);
>
> Style: Align open paren with other functions.
Done.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 64:
>
>> 62: void apply_summary_diff(VMATree::SummaryDiff diff);
>> 63: RegionsTree* tree() { return &_tree; }
>> 64: class Instance : public AllStatic {
>
> Style: Newline to separate the new class to the getter above.
Done.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 67:
>
>> 65: friend class VirtualMemoryTrackerTest;
>> 66: friend class CommittedVirtualMemoryTest;
>> 67: friend class ReservedMemoryRegion;
>
> Why is this necessary?
Not anymore. Left over from initial tests.
Removed.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 69:
>
>> 67:
>> 68: private:
>> 69: static RegionsTree* _tree;
>
> If you do the instance/static pattern then you can switch this into a regular member, no pointer necessary.
The `_tree` ctor to be called in `initialize` with a `bool` depending the `NMT_Level`. Existing VMT also uses a pointer to a `SortedLinkList`.
I am not sure yet: What is the advantage of not-a-pointer member? How the instance/static helps this?
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 69:
>
>> 67: friend class ReservedMemoryRegion;
>> 68:
>> 69: private:
>
> Unnecessary, remove.
Done.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 70:
>
>> 68: private:
>> 69: static RegionsTree* _tree;
>> 70: static NativeCallStackStorage* _ncs_storage;
>
> Doesn't seem like this is ever initialized?
Removed.
> src/hotspot/share/nmt/virtualMemoryTrackerWithTree.hpp line 77:
>
>> 75:
>> 76: static bool add_reserved_region (address base_addr, size_t size, const NativeCallStack& stack, MEMFLAGS flag = mtNone);
>> 77:
>
> Style: Extra space
The open paren should've been aligned with other functions.
It is aligned now.
> src/hotspot/share/nmt/vmatree.cpp line 33:
>
>> 31:
>> 32: const char* VMATree::statetype_strings[4] = {
>> 33: "released","reserved", "'only-committed", "committed",
>
> Extra quote in `"only-committed"`. Why is this new state required by the way?
Removed the extra quote.
Do you remember the new bit flags for state?
0 = Released, 1 = Reserved, 2= Committed(but not Reserved),
3= both Reserved and Committed
> src/hotspot/share/nmt/vmatree.cpp line 36:
>
>> 34: };
>> 35:
>> 36: void VMATree::put_if_absent(position A, position B, StateType state, const RegionData& region_data) {
>
> Unused.
Removed.
> src/hotspot/share/nmt/vmatree.hpp line 40:
>
>> 38: // or from committed memory of a certain MEMFLAGS to committed memory of a different MEMFLAGS.
>> 39: // The set of points is stored in a balanced binary tree for efficient querying and updating.
>> 40: class VMATree : public AnyObj {
>
> Why is it AnyObj now?
Mainly to be able to use `new operator`. Changed to `CheapObj<mtNMT>`.
> src/hotspot/share/nmt/vmatree.hpp line 41:
>
>> 39: // or from committed memory of a certain MemTag to committed memory of a different MemTag.
>> 40: // The set of points is stored in a balanced binary tree for efficient querying and updating.
>> 41: class VMATree : public CHeapObj<mtNMT> {
>
> Is this still needed?
Not anymore. Removed.
> src/hotspot/share/nmt/vmatree.hpp line 57:
>
>> 55: };
>> 56:
>> 57: enum class StateType : uint8_t { Reserved = 1, Committed = 3, Released = 0, COUNT = 3 };
>
> Why not the following?
>
> ```c++
> enum class StateType : uint8_t { Released, Reserved, Committed, COUNT };
>
>
> If this particular ordering is necessary then it should be documented why.
Done.
> src/hotspot/share/nmt/vmatree.hpp line 59:
>
>> 57: // Bit fields view: bit 0 for Reserved, bit 1 for Committed.
>> 58: // Setting a region as Committed preserves the Reserved state.
>> 59: enum class StateType : uint8_t { Reserved = 1, Committed = 3, Released = 0, COUNT = 4 };
>
> Why do we now consider `StateType` to be a bit field?
As the comments above it says, and we already had a discussion on it: The 'Committed' state now means 'Reserved and Committed'. So, when we visit nodes and check for Reserved ones, the committed nodes are also considered as Reserved. Otherwise, we would ignore the Committed nodes and continue looking for Reserved nodes.
> src/hotspot/share/nmt/vmatree.hpp line 234:
>
>> 232: }
>> 233:
>> 234: void put_if_absent(position A, position B, StateType state, const RegionData& region_data);
>
> Not used.
Removed.
> src/hotspot/share/nmt/vmtCommon.cpp line 1:
>
>> 1: /*
>
> Is this all of the code that is still required by the reporter to be able to function?
Actually, it is the old `virtualMemoryTracker.hpp` that now is renamed and refined to have only common stuff for the NMT. This is included in 13 files, not only the reporter.
> src/hotspot/share/nmt/vmtCommon.hpp line 40:
>
>> 38:
>> 39:
>> 40:
>
> A lot of excessive spaces.
Removed.
> src/java.base/share/classes/java/lang/String.java line 2990:
>
>> 2988: return this;
>> 2989: }
>> 2990: return StringConcatHelper.doConcat(this, str == null ? "null" : str);
>
> Remove
Done. It was a merge glitch.
> test/hotspot/gtest/nmt/test_nmt_treap.cpp line 168:
>
>> 166: tty->print_cr("d1: %lf, d2: %lf, d2/d1: %lf", d1, d2, d2 / d1);
>> 167: EXPECT_LT((int)(d2 / d1), N2 / N1);
>> 168: }
>
> 1000 and 10_000 elements are both quite small, isn't one measurement prone to being slower than expected due to unforeseen circumstances? For example, if `malloc` performs a heavy allocation (by mapping in a few new pages) during the 10_000 elements insertion then that may stall enough that the `EXPECT_LT` fails. We are also bound to only performing the test once.
>
> Is your thinking that these perf tests should be kept in, or should they be removed before integrating?
Generally, I think it is necessary to have performance tests to verify if any future changes do not degrade the performance.
The approach of stress testing can be agreed between us, but the tests have to exist.
In this approach, the input size is scaled N times and we expect the execution time grows as O(log(N)) which is much less than N.
This test fails and we need to have a justification for it. If approach is invalid or not correct implemented, we fix it. But the expectations remains the same.
> test/hotspot/gtest/nmt/test_vmatree.cpp line 368:
>
>> 366: }
>> 367:
>> 368: TEST_VM_F(NMTVMATreeTest, SummaryAccounting_dup) {
>
> Better name for the test, please avoid underscores in test names. I know, we have underscores in places in Hotspot, but it's a good rule for GTest naming.
Done.
> test/hotspot/jtreg/runtime/NMT/VirtualAllocCommitMerge.java line 123:
>
>> 121: checkCommitted(output, addrC, commitSize, "128KB");
>> 122: checkCommitted(output, addrE, commitSize, "128KB");
>> 123: System.out.println(output.getStdout());
>
> Shouldn't be here?
Removed.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2378894979
PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2410539689
PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2410730815
PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2411241108
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1778343343
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799016774
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799017291
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799018897
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1778343610
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799027228
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1803165698
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770961200
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770961642
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770962435
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703291746
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703292159
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799028731
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770964698
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713717938
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713466616
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713467548
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770965082
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713469220
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770966862
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770967338
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713563471
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1803163360
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713537731
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799031692
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799031982
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713721268
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770975913
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703295371
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770978952
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703295725
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703296483
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770981600
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770982361
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770986444
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1704419039
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770987209
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1713454670
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770988800
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799036588
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799036836
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703314202
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799037302
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1704419263
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1770973856
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799038284
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799059723
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799038564
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703298094
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703300827
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1703303785
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1799038857
More information about the core-libs-dev
mailing list