the result" would be better.
Best regards,
Andrej Golovnin
ls to zero.
Best regards,
Andrej Golovnin
On Tue, Sep 18, 2018 at 7:53 PM Jim Laskey wrote:
>
> Please review the code for String::detab and String::entab. Used to expand
> tabs into spaces, and spaces back to tabs.
>
> webrev: http://cr.openjdk.java.net/~jlaskey/8210717/webrev/ind
return (Set)Set.of(new HashSet<>(coll).toArray());
}
Best regards,
Andrej Golovnin
> Hi all,
>
> the current implementation of Set.copyOf(Collection coll)
> creates an intermediate HashSet object to remove duplicates even when
> coll is already a Set. The attached patch adds an additional check
> whether coll is already a Set and if yes, then calls #toArray()
> directly on coll a
what's went wrong.
Best regards,
Andrej Golovnin
/constant/SymbolicRefTest.java
test/jdk/java/lang/invoke/constant/TypeDescriptorTest.java
And I also think that Constable.describeConstable() is weirdly named.
What about:
#describe()
#getDescriptor()
#getConstantDescriptor()
#toConstantDescriptor()
Best regards,
Andrej Golovnin
On Wed, May 23, 2018
defined on the CharSequence interface with default
implementations and not on the String class. The sub-classes of
CharSequence shall then provide optimised implementations. This would
allow us to have this methods on StringBuilder and CharBuffer too.
Best regards,
Andrej Golovnin
On Wed, Apr 25, 2018
Hi David,
+if (! Modifier.isStatic(m.getModifiers())) {
I think the whitespace after the ‘!’-sign should be removed.
Best regards,
Andrej Golovnin
> On 14. Mar 2018, at 19:09, David Lloyd wrote:
>
> On Wed, Mar 14, 2018 at 1:00 PM, mandy chung wrote:
>> Thanks
better
as Class does not have own implementations of #hasCode() and
#equals() methods.
Best regards,
Andrej Golovnin
On Thu, Mar 1, 2018 at 1:14 PM, Claes Redestad
wrote:
> Hi,
>
> two trivial optimizations that get rid of a few percent on ISC startup
> tests.
>
> Webrev: http://cr
2 static void VERIFY(String result, String string, int repeat) {
Why is the method name in uppercase?
Best regards,
Andrej Golovnin
On Wed, Feb 28, 2018 at 5:31 PM, Jim Laskey wrote:
> Introduction of a new instance method String::repeat to allow an efficient
> and concise approach fo
mplicit nullcheck of o
602 }
I think that the comment about the implicit null check in the line 601
is not needed anymore.
Best regards,
Andrej Golovnin
f this problem
and I’m sure that you would provide the best possible solution. :-)
Best regards,
Andrej Golovnin
problem, then
I’m fine with it. When not, then let us debate about an alternative solution.
:-)
Best regards,
Andrej Golovnin
to mention, that Set12, SetN, Map12 and MapN should also
have specialised implementations for the #forEach-methods and for
the #spliterator()-methods in Set12, SetN.
Thanks!
Best regards,
Andrej Golovnin
would like to use the SubList class:
115 int size;
The size field should be final.
Thanks!
Best regards,
Andrej Golovnin
be much better. In any case please take look at
the implementation of the #lastIndexOf() method in the
AbstractImmutableList class. It looks like a copy of
AbstractImmutableList#indexOf() and this is wrong.
Best regards,
Andrej Golovnin
:
/**
* Returns the {@code Class} object of all the elements of this set.
*
* @return the {@code Class} object of all the elements of this set.
*
* @since 10
*/
public Class elementType()
The suggested change is attached as diff.
Best reagrds,
Andrej Golovnin
> On 15/11/17 10:03, Andrej Golovnin wrote:
>> I think we would need to write ugly code in any case as Java 9 has now
>> two empty list implementations: Collections.emptyList() and List.of().
>>
>> Collections.emptyList().sort() does not throw an exception.
>&
ould need to write ugly code in any case as Java 9 has now
two empty list implementations: Collections.emptyList() and List.of().
Collections.emptyList().sort() does not throw an exception.
List.of().sort() throws an exception.
Best regards,
Andrej Golovnin
>
> For singleton list I can kin
of ThreadPoolExecutor.finalize you can override
ThreadPoolExecutor.terminated.
Best regards
Andrej Golovnin
Hi Sherman,
thanks for the update. It looks much better now.
One more thing:
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipCoder.java
70 private static ZipCoder utf8 = new UTF8();
I think this field can be final as it never changes.
Best regards,
Andrej Golovnin
> On 18 Jan 2017, at 19
return new ZipCoder(cs);
79 } catch (Throwable t) {
80 t.printStackTrace();
81 }
82 return new ZipCoder(Charset.defaultCharset());
83 }
Wouldn't it be better to use System.Logger instead of printStackTrace
in the line 80?
Best regards,
Andrej G
ink the test should also check that all public methods
throw exceptions as described in the JavaDocs.
Best regards,
Andrej Golovnin
> On 10 Dec 2016, at 02:02, Vincent Ryan wrote:
>
> Thanks to those who provided review comments.
>
> I have incorporated most of them and update
Hi,
src/jdk.jartool/share/classes/sun/tools/jar/Main.java
682 if (info != null) {
683 info.print(out);
684return true;
685 }
The indentation in the line 684 is wrong: it uses 3 spaces instead of 4.
Best regards,
Andrej
mp; c != '>' && c != '<' &&
272!Character.isWhitespace(c);
273 c = current())
274 {
is easier to read than the long or-condition which is then negated?
Best regards,
Andrej Golovnin
On Tue, Nov 29, 2016 at 11:41 PM,
Hi Claes,
76 public static final int CLASS_NAME_SB_SIZE = 48;
Why is this constant public?
Btw. for our product this value is too small. We have packages with
longer names. I would use 64. :-)
Best regards,
Andrej Golovnin
On Tue, Nov 29, 2016 at 2:18 PM, Claes Redestad
wrote:
>
from Alan Bateman.
> The revised webrev is here:
>
> http://cr.openjdk.java.net/~smarks/reviews/8134373/webrev.03/
looks good. Thanks!
Best regards,
Andrej Golovnin
>
> In any case, yes, the specifications of the ResourceBundle.Control fields
> should be changed to remove the li
ants at all because they are part of the Public API and existed
in this form for a long time. Please ask someone from Oracle for help.
They can explain it better when it is OK to change and when not. Maybe
Stuart can do that.
Best regards,
Andrej Golovnin
Hi Stuart,
> Webrev:
>
> http://cr.openjdk.java.net/~smarks/reviews/8152617/webrev.0/
267 public Optional flatMap(Function> mapper)
I think there should be a space between “public” and “”.
Best regards,
Andrej Golovnin
Hi Patrick,
looks good for me. Thanks!
Best regards,
Andrej Golovnin
the stream */
2612 private int totalBytesRead = 0;
I think the type of the field totalBytesRead must be long. In the
#skip(long)-method you update it with a long value.
Best regards,
Andrej Golovnin
at compile
> time not on each method call.
As explained by Paul, the bitwise OR-operator is evaluated by the Java compiler
during the compilation time. Therefore there is no penalty at runtime if you
define the constant inside the method.
Best regards,
Andrej Golovnin
ore I think
the JavaDocs do not reflect what the method does.
And one more thing. Because we have now only one method to get a
stream I think the constant RESOURCE_CHARACTERISTICS should be defined
inside the #resources()-method. It is not needed to define it as a
static final field.
Best regards,
A
indentation is broken. You use tabs. But in JDK you must use
spaces, see
http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html#toc-indentation
Best regards,
Andrej Golovnin
On Thu, Sep 1, 2016 at 10:06 PM, Patrick Reinhart wrote:
> Hi Alan,
> Hi Paul,
>
> Here is the first
And if you add a method #getName() to the Name class,
then you can use method references in the lines 604 and 614 too.
+1 to Tagir's suggestions.
Best regards,
Andrej Golovnin
d override the fillInStackTrace()-method and make it empty. And
later in the class LambdaForm I would just print the cause of
BytecodeGenerationException.
src/java.base/share/classes/java/lang/invoke/LambdaForm.java
In the line 739 the field LF_FAILED should be final.
Best regards,
Andrej Golovnin
to raise an error in such cases.
I mean if IntelliJ is able to give you a hint that you use Optional.get()
without the isPresent()-check, then the Java compiler should be able
to do it too.
Just my two cents.
Best regards,
Andrej Golovnin
treamEx! Great job! :-)
As an average Java developer and an user of the Stream API and
the Optional class, I vote against deprecating of the Optional.get()-method.
(We have a democracy here, haven't we? :-D)
Best regards,
Andrej Golovnin
return hostClass.getName().replace(".", "/") +
"$$StringConcat”;
Maybe you should use here the character based String#replace()-method as it is
faster and
does not produce as much garbage as the CharSequence based method does.
Best regards,
Andrej Golovnin
> Apologies, I confused you with another Andrey :)
>
> Sure, let's do class-only:
> http://cr.openjdk.java.net/~shade/8148730/webrev.02/
No problem, Aleksey. Looks much better now. :)
Best regards,
Andrej Golovnin
rs added in a later version than the class. This
minimizes the number of @since tags.
Best regards,
Andrej Golovnin
Hi Aleksej,
is it really needed to add the "@since 9" tag to the
methods/constructors when the whole class is since JDK 9? As far as I
know the @since tag should be only added to class members introduced
in a later version than the class.
Best regards,
Andrej Golovnin
On Mon, Feb 1,
Hi Ivan,
> Please review the upcoming API changes:
> http://cr.openjdk.java.net/~ikrylov/8147844.jdk.00/
I think the semicolon at the end of the line 887 is not needed, e.g:
887 public static void onSpinWait() {};
should be
887 public static void onSpinWait() {}
Best regards,
+) {
1344 cls[i] = int.class;
1345 }
The for-loop can be replaced by:
Arrays.fill(cls, int.class);
Best regards,
Andrej Golovnin
);
The result should be the same. But it is easier to read.
Otherwise looks good. And I think you need now a real reviewer. :-)
Best regards,
Andrej Golovnin
MethodType,
WrongMethodTypeException, LambdaConversioinException). Does it have
some special meaning? I'm just curious.
Best regards,
Andrej Golovnin
ill not work as expected.
Best regards,
Andrej Golovnin
ppend((String) null);
The first one requires to copy byte values from the "null" String. The
second one ends up in the call to AsbtractStringBuilder.appendNull().
Best regards,
Andrej Golovnin
ate-tests.sh
The files do not have a copyright header.
Best regards,
Andrej Golovnin
Hi Paul,
wouldn't it be better to use Objects#equals(Object, Object) in the
line 3293 in the Arrays class instead of the ternary operator (the
same applies to the line 3238)?
Best regards,
Andrej Golovnin
On Mon, Oct 12, 2015 at 9:31 AM, Paul Sandoz wrote:
>
>> On 22 Sep 2015,
tic int[][] byteTables = new
int[8][256];
65 private transient final static int[] byteTable;
66 private transient final static int[] byteTable0 = byteTables[0];
Best regards,
Andrej Golovnin
e=all,file=$JCOV_OUT,merge=merge
where jcov_jdk_lang.txt contains the single line:
sun.reflect.*
> I think this patch is good to go. I need to file some Oracle internal
> requests, should take about a week, then I can sponsor this.
I am very pleased to hear that and I hope to contribute more.
Best regards,
Andrej Golovnin
;)
should be changed to:
316 sb.append("Extension unknown: DER encoded
OCTET string =\n")
same problems are in
src/java.base/share/classes/sun/security/x509/X509CRLImpl.java in the line
576
src/java.base/share/classes/sun/security/x509/X509CertInfo.java in the line
332
Best regards,
Andrej Golovnin
Hi Martin,
you are right. And in the line 297:
297 sb.append(getClass().getName()).append('
').append(Integer.toString(hashCode()));
Integer.toString() can be removed too.
Best regards,
Andrej Golovnin
On Thu, Aug 21, 2014 at 3:26 PM, Martin Desruisseaux <
mar
indentation. I looked only
at src/java.desktop/share/classes/javax/swing/RepaintManager.java.
Best regards,
Andrej Golovnin
>
>http://cr.openjdk.java.net/~weijun/8038277/core/webrev.00/
>http://cr.openjdk.java.net/~weijun/8038277/extra/webrev.00/
>
> --Max
>
> On
urity/cert/X509CertSelector.java
src/share/classes/javax/crypto/CryptoPermission.java
src/share/classes/javax/management/relation/Role.java
In src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jj in the line 423 a dot
is missed before append:
423 {jjtn000.name.append( '.')append(t.image); }
nice opportunity to create an RFE
for the Javac team. Following code:
Object o1 = ...;
Object o2 = ...;
String s = "abc" + o1 + "c" + o2 + "\n";
should be translated to:
String s = new
StringBuilder().append("abc").append(o1).append('c
Hi Otávio,
please ignore the previous diff. I'm sorry, there was a small mistake. I
have attached the corrected version.
Best regards,
Andrej Golovnin
On Mon, Aug 11, 2014 at 1:55 PM, Andrej Golovnin
wrote:
> Hi Otávio,
>
> About the template in Parser.jjt, TokenMgrError.java
ects/javacc/lists
on the dev or users mailing lists and send a message to JavaCC project. I'm
sure the developers of JavaCC would help you.
Best regards,
Andrej Golovnin
> About readable of code I just renamed this class to sb instead of buf,
> strbuf, etc. Because using StringBuilder bey
iName).append(']');
272 }
273
274 return "Extension " + type + ", server_name: " + sb;
275 }
to:
268 public String toString() {
269 StringBuilder sb = new StringBuilder();
270 sb.append("Extension ").append
.append(iexp).toString();
But I'm not sure how common is the usage of #hexDouble-method. Maybe this
change is not worth it.
Best regards,
Andrej Golovnin
On Mon, Jul 14, 2014 at 2:23 PM, Claes Redestad
wrote:
> Hi again,
>
> updated webrev: http://cr
but maybe you can change the line
3781 exp.append("0").append(len - 1);
to use the character-based API to append a single character.
Best regards,
Andrej Golovnin
On Mon, Jul 14, 2014 at 12:07 PM, Claes Redestad
wrote:
> Hi,
>
> please review t
l.java
src/share/classes/sun/reflect/UnsafeQualifiedStaticIntegerFieldAccessorImpl.java
src/share/classes/sun/reflect/UnsafeStaticIntegerFieldAccessorImpl.java
This classes will be fixed by the patch for the issue 5043030.
Otherwise it looks good.
Best regards,
Andrej Golovnin
On Fri, Jul 11, 20
d you please submit the ASM related changes to the ASM project
[1]? Thanks!
Best regards,
Andrej Golovnin
[1] http://asm.ow2.org/
Hi Pavel,
> As per [1] I'm updating this thread with yet another webrev:
>
> http://cr.openjdk.java.net/~prappo/8048267/webrev.05
Thanks and looks good.
Best regards,
Andrej Golovnin
d into JDK next time. I hope
you didn't committed the patch for the Longs yet. There were also changes
to the classes in the package "jdk.internal.org.objectweb.asm".
Best regards,
Andrej Golovnin
On Tue, Jul 1, 2014 at 2:39 PM, Pavel Rappo wrote:
> Otavio,
>
> As wi
work on it.
Otherwise it looks good to me.
Best regards,
Andrej Golovnin
Hi Otávio,
About Andrej, is it not possible add two people in "Contributed-by:" tag?
>
Thanks! But it's not needed. It's your contribution. I just help to review
the changes.
Best regards,
Andrej Golovnin
p here the code a
little bit.
>
> P.S. Andrej, it looks like you don't have an 'Author' status. Well, that's a
> pity. We could mention you in the 'Reviewed-by' line. Your contributions are
> really good.
Thanks! But I don't really care about it as long as I can help to improve the
overall code quality.
Best regards,
Andrej Golovnin
n Long.valueOf(pName).hashCode();
}
The method Long.hashCode(long) (added in JDK 8) should be used to calculate
the hash for a long value, e.g.:
+return Long.hashCode(pName);
Best regards,
Andrej Golovnin
On Fri, Jun 27, 2014 at 12:00 PM, Pavel Rappo
wrote:
> I created an issu
Stamp)).longValue();
> +long lastTime = (Long.valueOf(tStamp)).longValue();
>
>
> Paul.
>
>
Wouldn't it be better to use here Long.parseLong(String)?
Long.valueOf(String) would always create a new object here, as expTime and
tStamp represents the time and they are never in the range [-128;127].
Best regards,
Andrej Golovnin
LC_MONETARY=de_DE.utf8
export LC_NUMERIC=de_DE.utf8
export LC_MEASUREMENT=de_DE.utf8
export LC_TIME=de_DE.utf8
make test TEST=jdk_lang
I have created patch to fix this problem. The patch is attached to this
mail.
Best regards,
Andrej Golovnin
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014
hing I think we should make the decision
whether we are going to drop the VM part or not. As I already said, I'm for
the consistent behavior and therefore against the dropping the VM part. But
I'm only the user of the JDK. Maybe other members of the core team could
share with us their opinion.
Best regards,
Andrej Golovnin
:set_name(mh(), name());
Therefore I would say the name is actually interned in terms of String.intern().
And you can compare the name of a method with == with interned Strings.
The same applies to a name of a class and to a name of a field too.
Please feel free to correct me.
Best regards,
getParameterTypes(),
2768 toRemove.getParameterTypes())) {
2769 methods[i] = null;
Best regards,
Andrej Golovnin
Hi,
any update?
Best regards,
Andrej Golovnin
On 29.05.2014, at 09:35, Joel Borggrén-Franck wrote:
> Hi,
>
> We need you send in the patches via the mailing list. Please reply to your
> mail with the diffs attached directly (not zipped for example).
>
> I’ll might have ti
em with Integer objects too. I brought the example
with Boolean objects just because at least in the theory we should have per
JVM only two instances of Boolean class. But in the reality we have to many
of them.
Best regards,
Andrej Golovnin
tion back to Java
by calling
JavaCalls::call_static(&boxed_value,
klass_handle,
vmSymbols::valueOf_name(),
valueOf_signature,
&args,
THREAD);
But maybe I misunderstood the implementation of JavaCalls.
Best regards,
Andrej Golovnin
in TLABs and are removed by GC when the use
case is finished, I would say we have allocated 9GB of Boolean objects
to much. Or do you see it differently?
Let me know, if you need more data or if I should write some test.
If don't want accept the patch, then it's OK. But in this case you
should close the issue 5043030 and explain why it won't be fixed.
Best regards,
Andrej Golovnin
Hi all,
as requested here are the patches as diffs.
Best regards,
Andrej Golovnin
On 28.05.2014, at 23:44, Andrej Golovnin wrote:
> Hi Joe,
>
> I have prepared a patch for the issue JDK-5043030.
> The patch consists of two parts: one for jdk and one for hotspot.
> You can f
As I wrote in my previous mail, the JDK patch contains
two regression tests for jtreg to verify my changes.
If you run this tests using current JDK9, than the tests will fail.
After applying my patches the tests executed without any problem.
Best regards,
Andrej Golovnin
>
> cheers
> /
it and I hope you can sponsor it.
Best regards,
Andrej Golovnin
Hi Otávio,
it would be nice, if you would not modify the classes
sun.reflect.UnsafeXXXFieldAccessorImpl.
This classes should be changed as a part of the fix for the issue JDK-5043030.
The patch for this issue is already in work.
Best regards,
Andrej Golovnin
On 24.05.2014, at 16:34, Otávio
n, since which version
of JDK/Hotspot it is supported, where it is implemented in JDK?
When I take look at a product I'm working on, I see a lot instances of
ArrayList$Itr objects,
which are created by for-each loops (we use JDK 7u51).
Thanks in advance!
Best regards,
Andrej Golovnin
Hi Rémi, Vitaly,
Thank you for your explanation and for the tips!
> if you see a lot of instances of ArrayList iterator it means that you have a
> code path that create an iterator and use it later and the JIT is not able to
> see the creation and the use in the same inlining horizon. As Vitaly
n, since which version
of JDK/Hotspot it is supported, where it is implemented in JDK?
When I take look at a product I'm working on, I see a lot instances of
ArrayList$Itr objects,
which are created by for-each loops (we use JDK 7u51).
Thanks in advance!
Best regards,
Andrej Golovnin
Hello Joe,
when you already going to change the AnnotationType class,
could you also please remove duplicate line (lines 115 and 121
are identical) in the for-loop in the constructor:
https://dl.dropbox.com/u/148428677/jdk8/AnnotationType/webrev.01/index.html
Thanks,
Andrej
On 26.03.2013, at 23
ge AccessorGenerator and its sub-classes to use
#valueOf()-methods.
What do you think about it?
Best regards
Andrej Golovnin
PS: here is the inlined patch
# HG changeset patch
# User golovnin
# Date 1335989617 -7200
# Node ID 13e3638f723582d1df0b7c45c6aa983d4c2eed17
# Parent cfd7602f5c5247d551290
which use ORM solutions).
The patch does not modify Unsafe-based FieldAccessors for double and float
fields as the wrapper classes does not provide caching for fields of this types.
Best regards
Andrej Golovnin
89 matches
Mail list logo
