Hi Carsten,
The fix looks good to me.
Thank you for the update!
I'm waiting for a reply and thumbs up from Staffan.
Thanks,
Serguei
On 11/2/15 10:54, Carsten Varming wrote:
Dear Staffan,
I updated the webrev at
http://cr.openjdk.java.net/~cvarming/jvmtiGen.02/
<http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen.02/>
I kept the verbose flag to avoid printing the fallback warnings.
Carsten
On Mon, Nov 2, 2015 at 8:34 AM, Carsten Varming <[email protected]
<mailto:[email protected]>> wrote:
Dear Staffan,
Stacktraces: I can see your point. I'll add them back unconditionally.
Warnings: The OpenJDK build gets a little noisy when you start
printing warnings. The fallback on failed includes in
hotspot/src/share/vm/trace/*.xml cause a lot of printing; see
below. That is why I introduced the verbose flag in the first
place. This is not a big deal as it pollutes only the build log,
but I would still prefer the program to be silent on normal
execution. Please let me know if you still prefer to discard the
verbose flag and print all warnings.
I'll update the webrev after your reply.
Thank you,
Carsten
Noisy warnings:
jvmtiGen warning: Include operation failed, reverting to fallback.
Resource error reading file as XML
(href='../../../closed/share/vm/trace/traceeventtypes.xml').
Reason:
/Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceeventtypes.xml
(No such file or directory)
jvmtiGen warning: Include operation failed, reverting to fallback.
Resource error reading file as XML
(href='../../../closed/share/vm/trace/traceevents.xml'). Reason:
/Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceevents.xml
(No such file or directory)
jvmtiGen warning: Include operation failed, reverting to fallback.
Resource error reading file as XML
(href='../../../closed/share/vm/trace/traceeventtypes.xml').
Reason:
/Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceeventtypes.xml
(No such file or directory)
jvmtiGen warning: Include operation failed, reverting to fallback.
Resource error reading file as XML
(href='../../../closed/share/vm/trace/traceevents.xml'). Reason:
/Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceevents.xml
(No such file or directory)
jvmtiGen warning: Include operation failed, reverting to fallback.
Resource error reading file as XML
(href='../../../closed/share/vm/trace/traceeventtypes.xml').
Reason:
/Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceeventtypes.xml
(No such file or directory)
jvmtiGen warning: Include operation failed, reverting to fallback.
Resource error reading file as XML
(href='../../../closed/share/vm/trace/traceevents.xml'). Reason:
/Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceevents.xml
(No such file or directory)
On Mon, Nov 2, 2015 at 2:37 AM, Staffan Larsen
<[email protected] <mailto:[email protected]>> wrote:
Hi Carsten,
Thanks for spending the time on this - this version looks a
lot cleaner.
I would prefer to always print the exception stack trace. The
reason is that if something goes wrong, it will likely happen
on a build server where it isn’t simple to find and insert the
extra ‘-verbose’ flag. Better to log as much information as
possible in case of failure so it’s easier to find the root cause.
That leaves warnings from the parser as the only usage of
-verbose. Are there any warnings presently? If not, I would
remove the -verbose flag altogether and always print the
warnings as well.
Thanks,
/Staffan
On 1 nov. 2015, at 20:48, Carsten Varming <[email protected]
<mailto:[email protected]>> wrote:
Dear Serguei and Steffan,
Sorry about the late reply. The office was a little busy Friday.
Steffan's suggestion is to remove all the exception handling
code sound reasonable. I am not a fan of the main method
throwing checked exceptions, so I wrapped all the exception
handling into a tiny handler that simply prints the exception
and exits with a non-zero value. I never cared much for the
stacktrace, so I put printing of the stacktrace under the
verbose flag.
Speaking of cleaning up, there is code in there to use Apache
xalan instead of the standard xslt processor due to an
ancient bug fixed in Java 1.5. I removed that as well.
There was also a comment about the field "document" being
package globally accessible due to: "ref'd by the
tree-adapter". I have no idea what that means, but changing
document to a local variable in main didn't break the build,
so I guess that comment is out-of-date. Please let me know if
I am missing something here.
There was also a use of java.io.PrintStream. That class
suppresses IOExceptions: "Unlike other output streams, a
PrintStream never throws an IOException", so I removed the
PrintStream as I want to get a failure when something goes wrong.
I have put a new webrev at
http://cr.openjdk.java.net/~cvarming/jvmtiGen.02/
<http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen.02/>
Carsten
