On 09/01/2012 07:11 PM, Ulf wrote:
Am 01.09.2012 03:15, schrieb Stuart Marks:
On 8/31/12 3:19 AM, Ulf Zibis wrote:
Stuart, much thanks for your detailed explanation. The as is
situation has not
been in question from my side, but anyway it seems, that it had
catalysed a new
solution, despite t
On 1/09/2012 5:42 AM, Mike Duigou wrote:
The changes look good to me.
I am starting to come into agreement with Remi though that unless a type
specific array can be created for situations like the E[] elements array in
ArrayDeque then it should be declared as Object[] array since that's what i
Am 01.09.2012 03:15, schrieb Stuart Marks:
On 8/31/12 3:19 AM, Ulf Zibis wrote:
Stuart, much thanks for your detailed explanation. The as is situation has not
been in question from my side, but anyway it seems, that it had catalysed a new
solution, despite that the additional break; could affect
On 8/31/12 3:19 AM, Ulf Zibis wrote:
Stuart, much thanks for your detailed explanation. The as is situation has not
been in question from my side, but anyway it seems, that it had catalysed a new
solution, despite that the additional break; could affect JIT optimization of
the enclosing loop. So
The changes look good to me.
I am starting to come into agreement with Remi though that unless a type
specific array can be created for situations like the E[] elements array in
ArrayDeque then it should be declared as Object[] array since that's what is
actually created.
Mike
On Aug 31 2012,
On 08/30/2012 06:45 PM, Stuart Marks wrote:
On 8/30/12 7:14 AM, Ulf Zibis wrote:
Am 30.08.2012 01:20, schrieb Stuart Marks:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
@SuppressWarnings("fallthrough") is put to suppress warnings
generated by
another switch/case statements
Can't you move it from met
Stuart, much thanks for your detailed explanation. The as is situation has not been in question from
my side, but anyway it seems, that it had catalysed a new solution, despite that the additional
break; could affect JIT optimization of the enclosing loop. So there should be an explaining
commen
On 08/30/2012 04:14 PM, Ulf Zibis wrote:
Am 30.08.2012 01:20, schrieb Stuart Marks:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
@SuppressWarnings("fallthrough") is put to suppress warnings
generated by
another switch/case statements
Can't you move it from method scope to there?
while (
On 8/30/12 7:14 AM, Ulf Zibis wrote:
Am 30.08.2012 01:20, schrieb Stuart Marks:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
@SuppressWarnings("fallthrough") is put to suppress warnings generated by
another switch/case statements
Can't you move it from method scope to there?
while (i >=
On 8/30/12 1:17 AM, Andrew Haley wrote:
On 08/29/2012 11:53 PM, Stuart Marks wrote:
On 8/29/12 8:48 AM, Andrew Hughes wrote:
But presumably [-Werror] would be removed when everything is warning free?
-Werror should not be the default for everyone building OpenJDK, who then
end up having to fix
Hi Remi,
On 8/30/2012 5:06 AM, Rémi Forax wrote:
On 08/29/2012 09:11 PM, Dan Xu wrote:
On 08/29/2012 08:27 AM, Joe Darcy wrote:
Hello,
On 8/29/2012 1:48 AM, Rémi Forax wrote:
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Am 30.08.2012 01:20, schrieb Stuart Marks:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
@SuppressWarnings("fallthrough") is put to suppress warnings generated by
another switch/case statements
Can't you move it from method scope to there?
while (i >= matchlen && !seencomma) {
Am 30.08.2012 08:23, schrieb Joe Darcy:
On 8/29/2012 7:07 PM, Stuart Marks wrote:
On 8/29/12 4:56 PM, Joe Darcy wrote:
On 8/29/2012 4:20 PM, Stuart Marks wrote:
The various SecurityConstants being used here are Strings.
Note that this is doing String comparisons using == which is usually a bu
Am 30.08.2012 01:03, schrieb Stuart Marks:
On 8/29/12 3:50 AM, Ulf Zibis wrote:
In FilePermission.java file, I make one change to its method
signature,
public Enumeration elements() ==> public Enumeration
elements()
Actually the whole method is synchronized. To make this more clear
Oops, sorry about the noise. I was wrong :-(
-Ulf
Am 30.08.2012 14:33, schrieb Ulf Zibis:
Am 30.08.2012 01:39, schrieb Dan Xu:
I have updated my fix with above suggestions. Thanks for your comments. The webrev is at
http://cr.openjdk.java.net/~dxu/7193406/webrev.04/. Thanks!
-Dan
- *
Am 30.08.2012 01:39, schrieb Dan Xu:
I have updated my fix with above suggestions. Thanks for your comments. The webrev is at
http://cr.openjdk.java.net/~dxu/7193406/webrev.04/. Thanks!
-Dan
- * @param action the action string.
+ * @param actions the action string.
I think you woul
On 08/30/2012 01:39 AM, Dan Xu wrote:
On 08/29/2012 12:11 PM, Dan Xu wrote:
On 08/29/2012 08:27 AM, Joe Darcy wrote:
Hello,
On 8/29/2012 1:48 AM, Rémi Forax wrote:
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for t
On 08/29/2012 09:11 PM, Dan Xu wrote:
On 08/29/2012 08:27 AM, Joe Darcy wrote:
Hello,
On 8/29/2012 1:48 AM, Rémi Forax wrote:
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for the extra trouble!
- Kurchi
On 8/28/12
On Aug 30, 9:17am, a...@redhat.com (Andrew Haley) wrote:
-- Subject: Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java
| -Werror is probably OK for Java code, but not for HotSpot.
|
| We GCC developers keep adding new warnings, so poor souls who keep
| up-to-date with new Fedora
On 08/29/2012 11:53 PM, Stuart Marks wrote:
>
>
> On 8/29/12 8:48 AM, Andrew Hughes wrote:
>> But presumably [-Werror] would be removed when everything is warning free?
>> -Werror should not be the default for everyone building OpenJDK, who then
>> end up having to fix or workaround issues which
On 8/29/2012 7:07 PM, Stuart Marks wrote:
On 8/29/12 4:56 PM, Joe Darcy wrote:
On 8/29/2012 4:20 PM, Stuart Marks wrote:
The original code was like this:
427 private static int getMask(String actions) {
...
435 // Check against use of constants (used heavily within
Hi Stuart,
On 30/08/2012 8:53 AM, Stuart Marks wrote:
On 8/29/12 8:48 AM, Andrew Hughes wrote:
But presumably [-Werror] would be removed when everything is warning
free?
-Werror should not be the default for everyone building OpenJDK, who then
end up having to fix or workaround issues which are
On 8/29/12 4:56 PM, Joe Darcy wrote:
On 8/29/2012 4:20 PM, Stuart Marks wrote:
The original code was like this:
427 private static int getMask(String actions) {
...
435 // Check against use of constants (used heavily within the JDK)
436 if (actions == Secur
On 8/29/2012 4:20 PM, Stuart Marks wrote:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
436 switch (actions) {
437 case SecurityConstants.FILE_READ_ACTION:
438 return READ;
Oops, you have reverted the change to use switch-on-Strings by
webrev.03. Why?
A fair qu
On 08/29/2012 12:11 PM, Dan Xu wrote:
On 08/29/2012 08:27 AM, Joe Darcy wrote:
Hello,
On 8/29/2012 1:48 AM, Rémi Forax wrote:
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for the extra trouble!
- Kurchi
On 8/28/12
On 8/29/12 4:36 AM, Ulf Zibis wrote:
436 switch (actions) {
437 case SecurityConstants.FILE_READ_ACTION:
438 return READ;
Oops, you have reverted the change to use switch-on-Strings by webrev.03. Why?
A fair question. I had instigated some internal conve
On 8/29/12 3:50 AM, Ulf Zibis wrote:
In FilePermission.java file, I make one change to its method
signature,
public Enumeration elements() ==> public Enumeration
elements()
Actually the whole method is synchronized. To make this more clear, I suggest:
798 public synchronized E
On 8/29/12 8:48 AM, Andrew Hughes wrote:
But presumably [-Werror] would be removed when everything is warning free?
-Werror should not be the default for everyone building OpenJDK, who then
end up having to fix or workaround issues which are nothing to do with them.
It's easy enough for those w
On 08/29/2012 08:27 AM, Joe Darcy wrote:
Hello,
On 8/29/2012 1:48 AM, Rémi Forax wrote:
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for the extra trouble!
- Kurchi
On 8/28/12 10:22 PM, Dan Xu wrote:
It is funny. :)
- Original Message -
> On 8/24/12 2:42 AM, Andrew Hughes wrote:
> > However, once the whole build is warning free, would it not be
> > preferable
> > to remove these and just set JAVAC_WARNINGS_FATAL=true when doing
> > development
> > builds?
> >
> > The problem I see is someone new comi
Hello,
On 8/29/2012 1:48 AM, Rémi Forax wrote:
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for the extra trouble!
- Kurchi
On 8/28/12 10:22 PM, Dan Xu wrote:
It is funny. :) I have searched all source codes under jdk
Am 29.08.2012 07:22, schrieb Dan Xu:
It is funny. :) I have searched all source codes under jdk and removed spaces
for the similar cases.
Please review the new version of change at,
http://cr.openjdk.java.net/~dxu/7193406/webrev.03/.
Thanks for your comment!
In class j.u.Collections you hav
Am 24.08.2012 21:07, schrieb Dan Xu:
On 08/23/2012 06:53 PM, David Holmes wrote:
I'm surprised that you need this:
426 @SuppressWarnings("fallthrough")
...
436 switch (actions) {
437 case SecurityConstants.FILE_READ_ACTION:
438 return READ;
Oops, y
Am 24.08.2012 02:12, schrieb Andrew Hughes:
In FilePermission.java file, I make one change to its method
signature,
public Enumeration elements() ==> public Enumeration
elements()
I am not sure whether it will cause an issue of backward
compatibility.
Please advise. Thanks!
Actuall
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for the extra trouble!
- Kurchi
On 8/28/12 10:22 PM, Dan Xu wrote:
It is funny. :) I have searched all source codes under jdk and
removed spaces for the similar cases.
Ple
Thanks for cleaning up those spaces Dan. The changes look fine.
Sorry for the extra trouble!
- Kurchi
On 8/28/12 10:22 PM, Dan Xu wrote:
It is funny. :) I have searched all source codes under jdk and removed
spaces for the similar cases.
Please review the new version of change at,
http://cr.
It is funny. :) I have searched all source codes under jdk and removed
spaces for the similar cases.
Please review the new version of change at,
http://cr.openjdk.java.net/~dxu/7193406/webrev.03/.
Thanks for your comment!
-Dan
On 08/28/2012 05:32 PM, Kurchi Hazra wrote:
Irony of the day -
Irony of the day - those changes were done by me!
(http://cr.openjdk.java.net/~khazra/7157893/webrev.02/) :D
They were probably a mistake/oversight. I guess the better way is
without those extra spaces. See
http://docs.oracle.com/javase/tutorial/java/javaOO/annotations.html.
If you have tim
I also thought the space was not needed. But when I made the changes, I
found that many similar codes had the space when two or more warning
types need to be suppressed. For example, java/util/Collections.java,
java/util/Arrays.java, java/util/ComparableTimSort.java, and etc. If
only one warni
I don't think you need the space before "unchecked" and the one after
"rawtypes" in lines 128 and 147 in
http://cr.openjdk.java.net/~dxu/7193406/webrev.02/src/share/classes/java/util/PropertyResourceBundle.java.sdiff.html.
- Kurchi
On 8/28/2012 4:57 PM, Dan Xu wrote:
Thanks for all your good
Thanks for all your good suggestions!
I have updated my changes, which revoke changes to makefiles and put
@SuppressWarnings outside methods instead of introducing local variables
for small methods.
The webrev is at http://cr.openjdk.java.net/~dxu/7193406/webrev.02/. Thanks!
-Dan
On 08/27/2
On 8/27/12 3:55 AM, Doug Lea wrote:
The underlying issue is that code size is one of the criteria
that JITs use to decide to compile/inline etc. So long as they do
so, there will be cases here and there where it critically
important to keep sizes small in bottleneck code. Not many,
but still enou
Vitaly Davidovich wrote:
I figured you did, but wanted to check. :)
So the perf hit was with c2 compilation? Were you able to check the
assembly (or enable inlining printing in hotspot) and see that lack of
inlining (and whatever further opto that enabled) was the difference by
simply adding
On 25/08/2012 8:05 AM, Omair Majid wrote:
On 08/23/2012 09:43 PM, Stuart Marks wrote:
However, there are several cases where the following occurs:
SUBDIRS_MAKEFLAGS += JAVAC_WARNINGS_FATAL=true
and this is **not** overridable on the command line.
But it is! Use:
make SUBDIRS_MAKEFLAGS=
On 08/23/2012 09:43 PM, Stuart Marks wrote:
> However, there are several cases where the following occurs:
>
> SUBDIRS_MAKEFLAGS += JAVAC_WARNINGS_FATAL=true
>
> and this is **not** overridable on the command line.
But it is! Use:
make SUBDIRS_MAKEFLAGS=""
instead of
SUBDIRS_MAKEFLAGS=""
On 8/24/12 2:42 AM, Andrew Hughes wrote:
However, once the whole build is warning free, would it not be preferable
to remove these and just set JAVAC_WARNINGS_FATAL=true when doing development
builds?
The problem I see is someone new coming to OpenJDK and not being able to
simply build it (with
Hi David,
Please see my response below. Thanks!
On 08/23/2012 06:53 PM, David Holmes wrote:
Hi Dan,
On 24/08/2012 7:46 AM, Dan Xu wrote:
Please review the fix of CR 7193406 at
http://cr.openjdk.java.net/~dxu/7193406/webrev/.
It cleans up warnings in the following java files.
src/share/clas
On 08/23/2012 05:17 PM, Andrew Hughes wrote:
- Original Message -
- Original Message -
Please review the fix of CR 7193406 at
http://cr.openjdk.java.net/~dxu/7193406/webrev/.
snip...
And it enables fatal warning flag in the following make file.
make/java/jar/Makefil
I figured you did, but wanted to check. :)
So the perf hit was with c2 compilation? Were you able to check the
assembly (or enable inlining printing in hotspot) and see that lack of
inlining (and whatever further opto that enabled) was the difference by
simply adding a local or two? I'm legitimate
Vitaly Davidovich wrote:
So it sounds like avoiding these locals is basically trying to work
around current compiler limitations, rather than something more fundamental.
If javac did even a smidgen of optimization, this problem would
also go away.
I'm also curious if someone has actually n
In response to a similar question a few months ago, Tom Rodriguez mentioned
that c2 is not really susceptible to these redundant stores when making
inlining decisions. I guess c1 (and interpreter of course) might be but
then in those cases one's not getting max optimization anyway.
John Rose ment
Rémi Forax wrote:
Hi Dan,
I'm not sure to like the fact that you introduce some local variables
just to get ride of some warnings given that Hotspot compilers are
sometimes sensitive to that.
I think this practice should be discussed on this list before committing
this changeset.
Yes. We
Hi all,
> On 08/23/2012 11:46 PM, Dan Xu wrote:
>>
>> Please review the fix of CR 7193406 at
>> http://cr.openjdk.java.net/~dxu/7193406/webrev/.
>>
>> It cleans up warnings in the following java files.
>>
>>src/share/classes/java/io/FilePermission.java
>>src/share/classes/java/util/ArrayDe
On 08/23/2012 11:46 PM, Dan Xu wrote:
Please review the fix of CR 7193406 at
http://cr.openjdk.java.net/~dxu/7193406/webrev/.
It cleans up warnings in the following java files.
src/share/classes/java/io/FilePermission.java
src/share/classes/java/util/ArrayDeque.java
src/share/classes/
- Original Message -
> Hi Dan,
>
> On 24/08/2012 7:46 AM, Dan Xu wrote:
> > Please review the fix of CR 7193406 at
> > http://cr.openjdk.java.net/~dxu/7193406/webrev/.
> >
> > It cleans up warnings in the following java files.
> >
> > src/share/classes/java/io/FilePermission.java
>
> I'm
- Original Message -
> On 8/23/12 5:12 PM, Andrew Hughes wrote:
> > Dan Xu wrote:
> >> Please review the fix of CR 7193406 at
> >> http://cr.openjdk.java.net/~dxu/7193406/webrev/.
> >> And it enables fatal warning flag in the following make file.
> >>
> >> make/java/jar/Makefile
> >
>
Hi Dan,
On 24/08/2012 7:46 AM, Dan Xu wrote:
Please review the fix of CR 7193406 at
http://cr.openjdk.java.net/~dxu/7193406/webrev/.
It cleans up warnings in the following java files.
src/share/classes/java/io/FilePermission.java
I'm surprised that you need this:
426 @SuppressWarnings(
On 8/23/12 5:12 PM, Andrew Hughes wrote:
Dan Xu wrote:
Please review the fix of CR 7193406 at
http://cr.openjdk.java.net/~dxu/7193406/webrev/.
And it enables fatal warning flag in the following make file.
make/java/jar/Makefile
Please don't do this, at least not unconditionally.
Right,
- Original Message -
>
>
> - Original Message -
> > Please review the fix of CR 7193406 at
> > http://cr.openjdk.java.net/~dxu/7193406/webrev/.
> >
>
> snip...
>
> > And it enables fatal warning flag in the following make file.
> >
> > make/java/jar/Makefile
> >
>
> Pl
- Original Message -
> Please review the fix of CR 7193406 at
> http://cr.openjdk.java.net/~dxu/7193406/webrev/.
>
snip...
> And it enables fatal warning flag in the following make file.
>
> make/java/jar/Makefile
>
Please don't do this, at least not unconditionally.
At the ver
60 matches
Mail list logo
