Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v4]
> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. Daniel D. Daugherty has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge branch 'master' into JDK-8307067 - Merge branch 'master' into JDK-8307067 - dholmes CR - change ':' to '.'. - 8307067: remove broken EnableThreadSMRExtraValidityChecks option - Changes: - all: https://git.openjdk.org/jdk/pull/13704/files - new: https://git.openjdk.org/jdk/pull/13704/files/a3357b9a..eae82759 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=02-03 Stats: 69279 lines in 798 files changed: 59166 ins; 4592 del; 5521 mod Patch: https://git.openjdk.org/jdk/pull/13704.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13704/head:pull/13704 PR: https://git.openjdk.org/jdk/pull/13704
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v4]
On Thu, 11 May 2023 04:08:37 GMT, David Holmes wrote: >> Daniel D. Daugherty has updated the pull request with a new target base due >> to a merge or a rebase. The incremental webrev excludes the unrelated >> changes brought in by the merge/rebase. The pull request contains four >> additional commits since the last revision: >> >> - Merge branch 'master' into JDK-8307067 >> - Merge branch 'master' into JDK-8307067 >> - dholmes CR - change ':' to '.'. >> - 8307067: remove broken EnableThreadSMRExtraValidityChecks option > > Marked as reviewed by dholmes (Reviewer). @dholmes-ora - Thanks for the re-review! - PR Comment: https://git.openjdk.org/jdk/pull/13704#issuecomment-1546665766
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v3]
On Wed, 10 May 2023 19:56:53 GMT, Daniel D. Daugherty wrote: >> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. > > Daniel D. Daugherty has updated the pull request with a new target base due > to a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into JDK-8307067 > - dholmes CR - change ':' to '.'. > - 8307067: remove broken EnableThreadSMRExtraValidityChecks option Marked as reviewed by dholmes (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13704#pullrequestreview-1421723530
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v3]
> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. Daniel D. Daugherty has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Merge branch 'master' into JDK-8307067 - dholmes CR - change ':' to '.'. - 8307067: remove broken EnableThreadSMRExtraValidityChecks option - Changes: - all: https://git.openjdk.org/jdk/pull/13704/files - new: https://git.openjdk.org/jdk/pull/13704/files/59f7a22e..a3357b9a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=01-02 Stats: 76881 lines in 1715 files changed: 50695 ins; 13224 del; 12962 mod Patch: https://git.openjdk.org/jdk/pull/13704.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13704/head:pull/13704 PR: https://git.openjdk.org/jdk/pull/13704
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v2]
On Thu, 4 May 2023 05:11:53 GMT, Robbin Ehn wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> dholmes CR - change ':' to '.'. > > Looks good, thanks! @robehn - Thanks for the re-review! - PR Comment: https://git.openjdk.org/jdk/pull/13704#issuecomment-1535240037
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v2]
On Tue, 2 May 2023 20:37:18 GMT, Daniel D. Daugherty wrote: >> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. > > Daniel D. Daugherty has updated the pull request incrementally with one > additional commit since the last revision: > > dholmes CR - change ':' to '.'. Looks good, thanks! - Marked as reviewed by rehn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13704#pullrequestreview-1412295770
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v2]
On Tue, 2 May 2023 20:37:18 GMT, Daniel D. Daugherty wrote: >> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. > > Daniel D. Daugherty has updated the pull request incrementally with one > additional commit since the last revision: > > dholmes CR - change ':' to '.'. This fix was tested with Mach5 Tier[1-8] when it was part of: https://github.com/openjdk/jdk/pull/13519 and is also being tested with Mach5 Tier[1-8] combined with the fix from JDK-8307068 and what remains in JDK-8305670. - PR Comment: https://git.openjdk.org/jdk/pull/13704#issuecomment-1532139191
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v2]
On Tue, 2 May 2023 08:04:52 GMT, David Holmes wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> dholmes CR - change ':' to '.'.
>
> src/hotspot/share/runtime/threadSMR.cpp line 828:
>
>> 826: if (java_thread != JavaThread::current()) {
>> 827: // java_thread is not the current JavaThread so we have to verify it
>> 828: // against the ThreadsList:
>
> Colon at the end of the comment seems odd
Nice catch! Fixed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13704#discussion_r1183022658
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v2]
On Mon, 1 May 2023 13:44:22 GMT, Coleen Phillimore wrote: >> Daniel D. Daugherty has updated the pull request incrementally with one >> additional commit since the last revision: >> >> dholmes CR - change ':' to '.'. > > Yes, this looks good and also trivial. @coleenp, @dholmes-ora and @sspitsyn - thanks for the reviews! I suppose there isn't any way to give @robehn credit for his reviews when this fix was part of https://github.com/openjdk/jdk/pull/13519. - PR Comment: https://git.openjdk.org/jdk/pull/13704#issuecomment-1532107167
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option [v2]
> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision: dholmes CR - change ':' to '.'. - Changes: - all: https://git.openjdk.org/jdk/pull/13704/files - new: https://git.openjdk.org/jdk/pull/13704/files/bfb24496..59f7a22e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13704.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13704/head:pull/13704 PR: https://git.openjdk.org/jdk/pull/13704
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option
On Thu, 27 Apr 2023 22:17:30 GMT, Daniel D. Daugherty wrote: > A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. Marked as reviewed by sspitsyn (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13704#pullrequestreview-1408752268
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option
On Thu, 27 Apr 2023 22:17:30 GMT, Daniel D. Daugherty
wrote:
> A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option.
One minor nit but otherwise good. Thanks.
src/hotspot/share/runtime/threadSMR.cpp line 828:
> 826: if (java_thread != JavaThread::current()) {
> 827: // java_thread is not the current JavaThread so we have to verify it
> 828: // against the ThreadsList:
Colon at the end of the comment seems odd
-
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13704#pullrequestreview-1408596858
PR Review Comment: https://git.openjdk.org/jdk/pull/13704#discussion_r1182218848
Re: RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option
On Thu, 27 Apr 2023 22:17:30 GMT, Daniel D. Daugherty wrote: > A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. Yes, this looks good and also trivial. - Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13704#pullrequestreview-1407611225
RFR: 8307067: remove broken EnableThreadSMRExtraValidityChecks option
A trivial fix to remove broken EnableThreadSMRExtraValidityChecks option. - Commit messages: - 8307067: remove broken EnableThreadSMRExtraValidityChecks option Changes: https://git.openjdk.org/jdk/pull/13704/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=13704&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8307067 Stats: 11 lines in 3 files changed: 0 ins; 7 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/13704.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13704/head:pull/13704 PR: https://git.openjdk.org/jdk/pull/13704
