> Please see the JBS issue for more details, but basically we have 8 different 
> kinds of internal VM JavaThreads (grouping the three types of CompilerThread 
> together) that all basically duplicated the logic for initializing (preparing 
> is the term we use for user-defined JavaThreads) and starting the new thread. 
> This common code can be factored out into static helpers in JavaThread.
> 
> This change does not look at the way the java.lang.Thread instance is created 
> - that will be handled by a separate RFE.
> 
> The semantics of the changes are not identical, but I don't believe there is 
> any observable change in behaviour. The scope of holding the Threads_lock has 
> been reduced, and we now create the JavaThread instances ("new 
> XXXThread(...)") outside of the lock. As far as I can see nothing in the 
> construction process needs to happen under the Threads_lock.
> 
> A few of the threads use a static `_instance` field to hold a reference to 
> the create JavaThread. This proved very difficult to handle, as logically the 
> field would need to be updated in the middle of the new factored-out method: 
> after setting all the fields but before releasing the newly started thread. I 
> eventually realized that in all but one case those `_instance` fields are 
> never used and so could be deleted. The one case remaining does not need to 
> be set as I just described, but can be set after the thread has started, as 
> the new thread does not examine it (arguably its existence is unnecessary).
> 
> The trickiest changes related to the CompilerThreads, so they need particular 
> scrutiny.
> 
> Testing: tiers 1-3
> 
> Thanks,
> David

David Holmes has updated the pull request incrementally with three additional 
commits since the last revision:

 - Rename vm_exit_on_thread_allocation_failure to vm_exit_on_osthread_failure
 - Adjust comment
 - Comments from PR review:
   - remove unnecessary new_thread NULL checks
   - adjust some comments
   - fix whitespace

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4629/files
  - new: https://git.openjdk.java.net/jdk/pull/4629/files/e547e0a5..8275b184

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4629&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4629&range=01-02

  Stats: 28 lines in 10 files changed: 1 ins; 0 del; 27 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4629.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4629/head:pull/4629

PR: https://git.openjdk.java.net/jdk/pull/4629

Reply via email to