Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v4]
On Fri, 3 May 2024 01:54:24 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with three additional > commits since the last revision: > > - update > - Revert "renamed current_thread to current" > >This reverts commit d5d614bcf0861466acd695296e974d2253f84c9f. > - Revert "renamed current_thread tp current" > >This reverts commit 4602632221044aa754a1bc8d11e7a3e9a0092590. Looks good. - Marked as reviewed by cjplummer (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18986#pullrequestreview-2084100945
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v4]
On Fri, 3 May 2024 01:54:24 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with three additional > commits since the last revision: > > - update > - Revert "renamed current_thread to current" > >This reverts commit d5d614bcf0861466acd695296e974d2253f84c9f. > - Revert "renamed current_thread tp current" > >This reverts commit 4602632221044aa754a1bc8d11e7a3e9a0092590. Ping. Can I get second review please. - PR Comment: https://git.openjdk.org/jdk/pull/18986#issuecomment-2136243867
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v4]
On Fri, 3 May 2024 01:54:24 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with three additional > commits since the last revision: > > - update > - Revert "renamed current_thread to current" > >This reverts commit d5d614bcf0861466acd695296e974d2253f84c9f. > - Revert "renamed current_thread tp current" > >This reverts commit 4602632221044aa754a1bc8d11e7a3e9a0092590. Looks good. - Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18986#pullrequestreview-2039319268
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v4]
> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method > > Testing: tier1-6 Alex Menkov has updated the pull request incrementally with three additional commits since the last revision: - update - Revert "renamed current_thread to current" This reverts commit d5d614bcf0861466acd695296e974d2253f84c9f. - Revert "renamed current_thread tp current" This reverts commit 4602632221044aa754a1bc8d11e7a3e9a0092590. - Changes: - all: https://git.openjdk.org/jdk/pull/18986/files - new: https://git.openjdk.org/jdk/pull/18986/files/46026322..9be24a4a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18986=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18986=02-03 Stats: 122 lines in 2 files changed: 0 ins; 0 del; 122 mod Patch: https://git.openjdk.org/jdk/pull/18986.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18986/head:pull/18986 PR: https://git.openjdk.org/jdk/pull/18986
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v3]
On Tue, 30 Apr 2024 23:48:02 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > renamed current_thread tp current ok, I'll revert last update and rename current_thread to current only in few places where new variable is introduced - PR Comment: https://git.openjdk.org/jdk/pull/18986#issuecomment-2091927214
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v3]
On Tue, 30 Apr 2024 23:48:02 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > renamed current_thread tp current Given the number of `current` renames (which distracts from the core change in this PR), and given that not all occurrences were renamed (only those that were touched), I think it would be best to leave the rename for a separate PR. - PR Comment: https://git.openjdk.org/jdk/pull/18986#issuecomment-2091868298
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v3]
On Tue, 30 Apr 2024 02:05:10 GMT, Serguei Spitsyn wrote: >> Looks like in JVMTI `current_thread` is more common (and `current` is >> usually used in runtime :) > > The plan is to unify this with the approach used by the Runtime team. Replaced all touched "current_thread" and "calling_thread" with "current" - PR Review Comment: https://git.openjdk.org/jdk/pull/18986#discussion_r1585718011
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v3]
> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method > > Testing: tier1-6 Alex Menkov has updated the pull request incrementally with one additional commit since the last revision: renamed current_thread tp current - Changes: - all: https://git.openjdk.org/jdk/pull/18986/files - new: https://git.openjdk.org/jdk/pull/18986/files/d5d614bc..46026322 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18986=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18986=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18986.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18986/head:pull/18986 PR: https://git.openjdk.org/jdk/pull/18986
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v2]
> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method > > Testing: tier1-6 Alex Menkov has updated the pull request incrementally with one additional commit since the last revision: renamed current_thread to current - Changes: - all: https://git.openjdk.org/jdk/pull/18986/files - new: https://git.openjdk.org/jdk/pull/18986/files/f472f669..d5d614bc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18986=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18986=00-01 Stats: 131 lines in 2 files changed: 0 ins; 1 del; 130 mod Patch: https://git.openjdk.org/jdk/pull/18986.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18986/head:pull/18986 PR: https://git.openjdk.org/jdk/pull/18986
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly
On Fri, 26 Apr 2024 22:59:43 GMT, Alex Menkov wrote: > Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method > > Testing: tier1-6 This looks good in general. Thank you for taking care about it. The Runtime team suggested to use the name `current` for the current thread. The plan is to unify the JVMTI code as well. So, I'd suggest to rename all occurrences of the `current_thread` that you touched to the `current`. It will be a good step in a right direction. I'm already doing it for some time. - PR Review: https://git.openjdk.org/jdk/pull/18986#pullrequestreview-2030037342
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly
On Sat, 27 Apr 2024 00:01:16 GMT, Alex Menkov wrote: >> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1976: >> >>> 1974: oop thread_obj = nullptr; >>> 1975: >>> 1976: jvmtiError err = >>> JvmtiEnvBase::get_threadOop_and_JavaThread(tlh.list(), target, current, >>> _thread, _obj); >> >> I think a good cleanup would be to also replace `current` with >> `current_thread`, although I'm not sure how common each are. I see 3 >> `current` references in this webrev. > > Looks like in JVMTI `current_thread` is more common (and `current` is usually > used in runtime :) The plan is to unify this with the approach used by the Runtime team. - PR Review Comment: https://git.openjdk.org/jdk/pull/18986#discussion_r1584032225
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly
On Fri, 26 Apr 2024 23:36:41 GMT, Chris Plummer wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > src/hotspot/share/prims/jvmtiEnvBase.cpp line 1976: > >> 1974: oop thread_obj = nullptr; >> 1975: >> 1976: jvmtiError err = >> JvmtiEnvBase::get_threadOop_and_JavaThread(tlh.list(), target, current, >> _thread, _obj); > > I think a good cleanup would be to also replace `current` with > `current_thread`, although I'm not sure how common each are. I see 3 > `current` references in this webrev. Looks like in JVMTI `current_thread` is more common (and `current` is usually used in runtime :) - PR Review Comment: https://git.openjdk.org/jdk/pull/18986#discussion_r1581628063
Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly
On Fri, 26 Apr 2024 22:59:43 GMT, Alex Menkov wrote: > Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method > > Testing: tier1-6 src/hotspot/share/prims/jvmtiEnvBase.cpp line 1976: > 1974: oop thread_obj = nullptr; > 1975: > 1976: jvmtiError err = > JvmtiEnvBase::get_threadOop_and_JavaThread(tlh.list(), target, current, > _thread, _obj); I think a good cleanup would be to also replace `current` with `current_thread`, although I'm not sure how common each are. I see 3 `current` references in this webrev. - PR Review Comment: https://git.openjdk.org/jdk/pull/18986#discussion_r1581610079
RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly
Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method Testing: tier1-6 - Commit messages: - fix Changes: https://git.openjdk.org/jdk/pull/18986/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18986=00 Issue: https://bugs.openjdk.org/browse/JDK-8330852 Stats: 42 lines in 3 files changed: 3 ins; 10 del; 29 mod Patch: https://git.openjdk.org/jdk/pull/18986.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18986/head:pull/18986 PR: https://git.openjdk.org/jdk/pull/18986
