Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. OK. I started with serviceability but then went through everything as it's hard to record what you have/haven't seen in these long reviews. test/jdk/java/net/InterfaceAddress/Equals.java line 81: > 79: /** > 80: * Returns an InterfaceAddress instance with its fields set the values > 81: * specificed. probably a typo for "set to the values specified." - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. test/jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java line 84: > 82: > 83: /** > 84: * Create a Zip file that will result in an Zip64 Extra (EXT) header "result in a Zip64..." test/jdk/sun/security/tools/jarsigner/OldSig.java line 32: > 30: /* > 31: * See also PreserveRawManifestEntryAndDigest.java for tests with > arbitrarily > 32: * formatted individual sections in addition the main attributes tested "in addition to the..." - PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. src/hotspot/share/cds/filemap.cpp line 1914: > 1912: > 1913: // the current value of the pointers to be patched must be within > this > 1914: // range (i.e., must be between the requested base address, and the > of the current archive). "the of the" ? Maybe "..and the address of the current archive", or maybe it was a typo for "and that of the current archive". - PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. src/hotspot/share/interpreter/bytecodeUtils.cpp line 186: > 184: static const int _max_cause_detail = 5; > 185: > 186: // Merges the stack the given bci with the given stack. If there "...the stack at the given bci..." - PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. src/hotspot/share/opto/graphKit.cpp line 3626: > 3624: // The optional arguments are for specialized use by intrinsics: > 3625: // - If 'extra_slow_test' if not null is an extra condition for the > slow-path. > 3626: // - If 'return_size_val', report the total object size to the caller. "reports the total" - PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote:
> Replaces usages of articles that follow each other in all combinations:
> a/the, an?/an?, the/the…
>
> Also, I fixed a couple of spelling mistakes.
src/jdk.jdi/share/classes/com/sun/jdi/ClassType.java line 348:
> 346:
> 347: /**
> 348: * Returns the single non-abstract {@link Method} visible from
I would think "Returns a single ..." because the implementation returns the
first match it finds, from possibly many.
-
PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. src/jdk.sctp/share/classes/com/sun/nio/sctp/ShutdownNotification.java line 28: > 26: > 27: /** > 28: * Notification emitted when a peers shutdowns the association. Maybe: "...when a peer shuts down an association." (could be "shuts down the association" if there is only ever one, but using "an" looks safe) - PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Been through the JDI changes and jdb/example code. Built and manually tested some operations with jdb observing virtual threads. Been through jdk.management/share/classes/com/sun/management and also manually tested jconsole attaching (e.g. dumpThreads operation in both TEXT_PLAIN and JSON). - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285366: Fix typos in serviceability
On Thu, 21 Apr 2022 17:22:04 GMT, Magnus Ihse Bursie wrote: >> src/jdk.jdwp.agent/share/native/libjdwp/invoker.h line 38: >> >>> 36: jboolean pending; /* Is an invoke requested? */ >>> 37: jboolean started; /* Is an invoke happening? */ >>> 38: jboolean available;/* Is the thread in an invocable state? */ >> >> invocable could perhaps stay as invokable ? >> Elsewhere we have a filename com/sun/tools/jdi/InvokableTypeImpl.java which >> we clearly don't want to change, and I see Internet dictionary evidence of >> invokable being acceptable. > > But on the other hand we have `javax.script.Invocable`. :-) > > Codespell suggested this change, and I based my decision to keep it based on > [Merriam-Webster](https://www.merriam-webster.com/dictionary/invocable) not > even listing "invokable" as an alternate spelling, and that "invocable" has > about 3x the number of Google hits than "invokable". > > But sure, there is perhaps reason to consider "invokable" an acceptable > alternative and keep it. I guess it depends on if you consider the word to be > based on "invoke" with a suffix, or a latinized form, like "invocation". > > I'll wait a while for others to chime in, otherwise I'll revert the > "invokable" changes. Sure, I just thought there was enough evidence that invokable is not definitely wrong, even if invocable is more correct and popular, so it's not obvious it should be changed. Don't lose sleep over it. 8-) More importantly, on the tests -- I see the changes in exception messages searched for the wrong text in the test directories, and didn't find any matches that looked like checks. - PR: https://git.openjdk.java.net/jdk/pull/8334
Re: RFR: 8285366: Fix typos in serviceability
On Thu, 21 Apr 2022 11:22:48 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by the serviceability team > (`java.instrument java.management.rmi java.management jdk.attach > jdk.hotspot.agent jdk.internal.jvmstat jdk.jcmd jdk.jconsole jdk.jdi > jdk.jdwp.agent jdk.jstatd jdk.management.agent jdk.management`), and accepted > those changes where it indeed discovered real typos. > > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. All looks good to me, just the invokable which you might want to leave as is, unless there are other strong feelings. 8-) src/jdk.jdwp.agent/share/native/libjdwp/invoker.h line 38: > 36: jboolean pending; /* Is an invoke requested? */ > 37: jboolean started; /* Is an invoke happening? */ > 38: jboolean available;/* Is the thread in an invocable state? */ invocable could perhaps stay as invokable ? Elsewhere we have a filename com/sun/tools/jdi/InvokableTypeImpl.java which we clearly don't want to change, and I see Internet dictionary evidence of invokable being acceptable. - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/8334
Re: RFR: 8284853: Fix various 'expected' typo [v2]
On Thu, 14 Apr 2022 18:04:16 GMT, Andrey Turbanov wrote: > I found [yet another > typo](https://github.com/kelthuzadx/jdk/commit/acb9e15bc0bf5395d1c0875f36992f692734f948) > ... I didn't think "JVMInvokeMethodSlack" was a typo. I think it's the idea of "slack space" meaning leftover space. We require a certain amount of this space. We need some slack on the stack, in order to invoke. - PR: https://git.openjdk.java.net/jdk/pull/8231
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. (adding a note to trigger email notification, as that appears lost...) - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:04:43 GMT, Pavel Rappo wrote:
>> src/java.sql/share/classes/java/sql/BatchUpdateException.java line 58:
>>
>>> 56: * A JDBC driver implementation should use
>>> 57: * the constructor {@code BatchUpdateException(String reason, String
>>> SQLState,
>>> 58: * int vendorCode, long []updateCounts, Throwable cause) } instead of
>>
>> While we are here, is it more normal to say "long[] updateCounts", not
>> separating the [] from the type.
>> Similarly at line 81, 118, 151, 247, 277, 318, 354.
>
> I thought about it too, but then noticed how the position of `[]` mimics that
> of the respective method signatures in code.
OK, so lines 264, 295, 329, 364, 431 are arguably wrong as well? Separating
the [] completely looks quite rare.
I'll leave it up to you. 8-)
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote: > - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - `` is an apostrophe, which does not require to be encoded. Marked as reviewed by kevinw (Committer). - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote:
> - Most of the typos are of a trivial kind: missing whitespace.
> - If any of the typos should be fixed in the upstream projects instead,
> please say so; I will drop those typos from the patch.
> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant
> formatting artefact and thus can be removed.
> - `` is an apostrophe, which does not require to be encoded.
src/java.sql/share/classes/java/sql/Statement.java line 784:
> 782: * statement returns a {@code ResultSet} object, the second argument
> 783: * supplied to this method is not an
> 784: * {@code int} array whose elements are valid column indexes, the
> method is called on a
Should it be "or the method is called on...", i.e. add the "or", otherwise it's
a list of problems but we don't know if they are all required, or are
alternatives. It probably does not mean that all these have to be true to
throw the exception, but it doesn't say that. We are nitpicking of course.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote:
> - Most of the typos are of a trivial kind: missing whitespace.
> - If any of the typos should be fixed in the upstream projects instead,
> please say so; I will drop those typos from the patch.
> - As I understand it, in ImageInputStream and DataInput is an irrelevant
> formatting artefact and thus can be removed.
> - is an apostrophe, which does not require to be encoded.
src/java.sql/share/classes/java/sql/BatchUpdateException.java line 58:
> 56: * A JDBC driver implementation should use
> 57: * the constructor {@code BatchUpdateException(String reason, String
> SQLState,
> 58: * int vendorCode, long []updateCounts, Throwable cause) } instead of
While we are here, is it more normal to say "long[] updateCounts", not
separating the [] from the type.
Similarly at line 81, 118, 151, 247, 277, 318, 354.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote: > - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - is an apostrophe, which does not require to be encoded. src/java.base/share/classes/sun/text/RuleBasedBreakIterator.java line 73: > 71: * will be transparent to the BreakIterator. A sequence of > characters will break the > 72: * same way it would if any ignore characters it contains are taken > out. Break > 73: * positions never occur before ignore characters. "before ignored characters" - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr
On Fri, 10 Dec 2021 07:00:10 GMT, Chris Plummer wrote: > The test searches for "JShellToolProvider" in the main thread's stack trace, > which is pulled from an SA heap dump. Typically the main thread is blocked in > Object.wait(), so SA can determine its stack trace. However, the wait has a > 100ms timeout, so the thread does periodically wake up and does a few things > before waiting again. If SA tries to get the stack trace while the thread is > active, it may not be able to, and this causes the test to fail. I determined > this only happens about 1 in 5000-1 runs. So as a fix I'm allowing the > test to do one retry. That should make it extremely unlikely that we ever see > this failure again. I also bumped up the amount of time the test waits before > doing the heap dump from 2 seconds to 4 just to make absolutely sure the main > thread is fully started before doing the heap dump. Looks good. - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v3]
On Thu, 1 Jul 2021 10:38:24 GMT, Сергей Цыпанов
wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8268764: Update copy-right year
Marked as reviewed by kevinw (Committer).
OK, one more (C) in
src/jdk.jdi/share/classes/com/sun/tools/jdi/ReferenceTypeImpl.java and done. 8-)
Needs a second Review before integrating, thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v2]
On Wed, 30 Jun 2021 11:49:51 GMT, Сергей Цыпанов
wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов 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 two additional
> commits since the last revision:
>
> - Merge branch 'master' into 8268764
> - 8268764: Use Long.hashCode() instead of int-cast where applicable
The changes look good to me, we have done the same thing elsewhere. This
changes things in different functional areas, which is maybe unusual, but seems
fine for a small change as long as nobody objects.
Some of the files also need a (C) year update.
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata
Thanks Martin - I think we're going to proceed with the arguably
somewhat ugly change as it does solve a real problem, although somewhat
unusual to have such large thread local sizes for it to matter. And we
may have to deal with a thread library that takes away stack space for
thread local data for a while.
Thanks
Kevin
On 25/02/2016 16:30, Martin Buchholz wrote:
My hope is that we eventually eliminate StackOverflowError by
implementing dynamically growable thread stacks.
Until then, my hope is that hotspot give the thread the memory it asks
for, for actually storing stack frames, and thread locals should not
steal space from that.
The system property introduced in this change is an ugly workaround for that.
On Thu, Feb 18, 2016 at 2:24 AM, Kevin Walls <kevin.wa...@oracle.com> wrote:
Hi Cheleswer,
Looks good to me.
Thanks
Kevin
(Also, as one of the comments was that there may be no real cost to using
default stack sizes here (on most systems...?), having
jdk.lang.processReaperUseDefaultStackSize gives us a way to test that
widely, and one day the 32k may be able to disappear.)
On 17/02/2016 15:17, cheleswer sahu wrote:
Hi,
I have made changes in the property name
(jdk.lang.processReaperUseDefaultStackSize) and code as suggested in the
earlier reviews.
--- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.08163 +0530
@@ -81,9 +81,8 @@
ThreadGroup systemThreadGroup = tg;
ThreadFactory threadFactory = grimReaper -> {
-// Our thread stack requirement is quite modest.
-Thread t = new Thread(systemThreadGroup, grimReaper,
-"process reaper", 32768);
+long stackSize =
Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") ? 0 : 32768;
+Thread t = new Thread(systemThreadGroup, grimReaper,
"process reaper", stackSize);
t.setDaemon(true);
// A small attempt (probably futile) to avoid priority
inversion
t.setPriority(Thread.MAX_PRIORITY);
I would really like to thanks "Martin" for the patch
(http://cr.openjdk.java.net/~martin/webrevs/openjdk9/tls-size-guarantee/)
which he has provided. Since we support a wider range of glibc versions and
platform using patch will
require more testing and work. I think the use of system property for this
issue will be safer at this point of time.
Regards,
Cheleswer
On 1/19/2016 5:40 PM, David Holmes wrote:
On 19/01/2016 9:53 PM, Kevin Walls wrote:
|
Hi Cheleswer, I think Martin is suggesting something like:
|
// Use a modest stack size, unless requested otherwise:
long stackSize = Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0
: 32768;
Someone from core-libs needs to chime on what the appropriate namespace for
such a property would be.
David
-
Thread t = new Thread(systemThreadGroup, grimReaper, "process reaper",
stackSize);
|||
If that tests OK for you, it may be the way we can go ahead with the
point fix for this.
Regards
Kevin
|
On 18/01/2016 16:52, Martin Buchholz wrote:
Historical note - I never liked having a reaper thread for each
subprocess, but no other reliable implementation is known. Given the
potential for many reaper threads, I at least wanted to keep the
memory waste low.
On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
<cheleswer.s...@oracle.com> wrote:
+ Thread t = null;
+ if
(Boolean.getBoolean("processReaperUseDefaultStackSize")) {
+ t = new Thread(systemThreadGroup, grimReaper,
"process reaper");
+} else {
+ // Our thread stack requirement is quite modest.
+ t = new Thread(systemThreadGroup, grimReaper,
"process reaper", 32768);
+}
If we do end up using this strategy, cleaner would be to use
https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#Thread-java.lang.ThreadGroup-java.lang.Runnable-java.lang.String-long-
stackSize - the desired stack size for the new thread, or zero to
indicate that this parameter is to be ignored.
Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata
Hi Cheleswer,
Looks good to me.
Thanks
Kevin
(Also, as one of the comments was that there may be no real cost to
using default stack sizes here (on most systems...?), having
jdk.lang.processReaperUseDefaultStackSize gives us a way to test that
widely, and one day the 32k may be able to disappear.)
On 17/02/2016 15:17, cheleswer sahu wrote:
Hi,
I have made changes in the property name
(jdk.lang.processReaperUseDefaultStackSize) and code as suggested in
the earlier reviews.
--- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.08163 +0530
@@ -81,9 +81,8 @@
ThreadGroup systemThreadGroup = tg;
ThreadFactory threadFactory = grimReaper -> {
-// Our thread stack requirement is quite modest.
-Thread t = new Thread(systemThreadGroup, grimReaper,
-"process reaper", 32768);
+long stackSize =
Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") ? 0 :
32768;
+Thread t = new Thread(systemThreadGroup,
grimReaper, "process reaper", stackSize);
t.setDaemon(true);
// A small attempt (probably futile) to avoid
priority inversion
t.setPriority(Thread.MAX_PRIORITY);
I would really like to thanks "Martin" for the patch
(http://cr.openjdk.java.net/~martin/webrevs/openjdk9/tls-size-guarantee/)
which he has provided. Since we support a wider range of glibc
versions and platform using patch will
require more testing and work. I think the use of system property for
this issue will be safer at this point of time.
Regards,
Cheleswer
On 1/19/2016 5:40 PM, David Holmes wrote:
On 19/01/2016 9:53 PM, Kevin Walls wrote:
|
Hi Cheleswer, I think Martin is suggesting something like:
|
// Use a modest stack size, unless requested otherwise:
long stackSize =
Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0 : 32768;
Someone from core-libs needs to chime on what the appropriate
namespace for such a property would be.
David
-
Thread t = new Thread(systemThreadGroup, grimReaper, "process
reaper", stackSize);
|||
If that tests OK for you, it may be the way we can go ahead with the
point fix for this.
Regards
Kevin
|
On 18/01/2016 16:52, Martin Buchholz wrote:
Historical note - I never liked having a reaper thread for each
subprocess, but no other reliable implementation is known. Given the
potential for many reaper threads, I at least wanted to keep the
memory waste low.
On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
<cheleswer.s...@oracle.com> wrote:
+ Thread t = null;
+ if
(Boolean.getBoolean("processReaperUseDefaultStackSize")) {
+ t = new Thread(systemThreadGroup, grimReaper,
"process reaper");
+} else {
+ // Our thread stack requirement is quite
modest.
+ t = new Thread(systemThreadGroup, grimReaper,
"process reaper", 32768);
+}
If we do end up using this strategy, cleaner would be to use
https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#Thread-java.lang.ThreadGroup-java.lang.Runnable-java.lang.String-long-
stackSize - the desired stack size for the new thread, or zero to
indicate that this parameter is to be ignored.
Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata
|
Hi Cheleswer, I think Martin is suggesting something like:
|
// Use a modest stack size, unless requested otherwise:
long stackSize = Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0 :
32768;
Thread t = new Thread(systemThreadGroup, grimReaper, "process reaper",
stackSize);
|||
If that tests OK for you, it may be the way we can go ahead with the
point fix for this.
Regards
Kevin
|
On 18/01/2016 16:52, Martin Buchholz wrote:
Historical note - I never liked having a reaper thread for each
subprocess, but no other reliable implementation is known. Given the
potential for many reaper threads, I at least wanted to keep the
memory waste low.
On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
wrote:
+ Thread t = null;
+ if
(Boolean.getBoolean("processReaperUseDefaultStackSize")) {
+ t = new Thread(systemThreadGroup, grimReaper,
"process reaper");
+} else {
+ // Our thread stack requirement is quite modest.
+ t = new Thread(systemThreadGroup, grimReaper,
"process reaper", 32768);
+}
If we do end up using this strategy, cleaner would be to use
https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#Thread-java.lang.ThreadGroup-java.lang.Runnable-java.lang.String-long-
stackSize - the desired stack size for the new thread, or zero to
indicate that this parameter is to be ignored.
hg: jdk7/tl/jdk: 6968933: Clip loop() deadlock in DirectAudioDevice$DirectClip.run
Changeset: d36ad8686f6d Author:kevinw Date: 2010-12-21 11:32 + URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/d36ad8686f6d 6968933: Clip loop() deadlock in DirectAudioDevice$DirectClip.run Reviewed-by: amenkov ! src/share/classes/com/sun/media/sound/DirectAudioDevice.java
Re: 6988037: fileClose prints debug message if close fails
Hi Alan,
That sounds familiar and looks good. (There has been some testing with
such a change already 8-) )
Regards
Kevin
On 29/09/2010 13:25, Alan Bateman wrote:
I need a reviewer for a small change to remove a debug message that
someone left when fixing a bug in the java.io implementation early in
jdk7. The debug message is printed if closing a file fails (for
example EIO because of a NFS stale handle). That change also added
code to restore the file descriptor field but this isn't needed
because the stream classes have a closed flag to ensure that they only
attempt to close the underlying file descriptor once. Furthermore, if
close fails then the state of the file descriptor is unspecified and
so cannot be used again. The proposed patch is attached.
Thanks,
Alan.
diff --git a/src/solaris/native/java/io/io_util_md.c
b/src/solaris/native/java/io/io_util_md.c
--- a/src/solaris/native/java/io/io_util_md.c
+++ b/src/solaris/native/java/io/io_util_md.c
@@ -83,8 +83,6 @@ fileClose(JNIEnv *env, jobject this, jfi
close(devnull);
}
} else if (JVM_Close(fd) == -1) {
-SET_FD(this, fd, fid); // restore fd
-printf(JVM_Close returned -1\n);
-JNU_ThrowIOExceptionWithLastError(env, close failed);
+JNU_ThrowIOExceptionWithLastError(env, close failed);
}
}
diff --git a/src/windows/native/java/io/io_util_md.c
b/src/windows/native/java/io/io_util_md.c
--- a/src/windows/native/java/io/io_util_md.c
+++ b/src/windows/native/java/io/io_util_md.c
@@ -531,7 +531,6 @@ handleClose(JNIEnv *env, jobject this, j
SET_FD(this, -1, fid);
if (CloseHandle(h) == 0) { /* Returns zero on failure */
-SET_FD(this, fd, fid); // restore fd
JNU_ThrowIOExceptionWithLastError(env, close failed);
}
return 0;
hg: jdk7/tl/jdk: 2 new changesets
Changeset: b8004f6f4812 Author:kevinw Date: 2009-09-22 17:01 +0100 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/b8004f6f4812 6882768: (launcher) test for 6842838 is broken Summary: Testcase correction. Reviewed-by: ksrini ! test/tools/launcher/6842838/Test6842838.sh Changeset: f708138c9aca Author:kevinw Date: 2009-09-22 17:16 +0100 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/f708138c9aca Merge
