Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi Brian,

On 2018-09-27 01:53, Brian Burkhalter wrote:

There was a compilation error on Linux in one of the tests:

test/jdk/java/lang/Floating/DoubleToDecString.java:133: error: unmappable 
character (0x93) for encoding US-ASCII
 Paxson V, "A Program for Testing IEEE Decimal\ufffd\ufffd\ufffdBinary 
Conversion"

This was only in one of the comments where it looks like some errant character 
leaked in. I updated the webrev (with accompanying patch) in place [1] after 
verifying that the change to the test fixes the problem.



I can confirm that in my original source there is a 'EN DASH' (U+2013) 
Unicode character, which visually looks similar to 'HYPHEN-MINUS' 
(U+002D). I use UTF-8 on all my source files, so it didn't stand out as 
something strange in the IDE.


This is certainly due to some copy&paste operation from a source found 
on the internet. U+2013 is the correct variant, both semantically and 
typographically, but it surely causes problems with tools that expect 
US-ASCII. Frankly, there should be no such restrictions in tools, as 
Java supports Unicode source code since day 0, but that's another story.


For this project, I will switch to US-ASCII in my IDE.




Also, there were a couple of failures observed in test [2] at lines 172 and 173. If at 
line 172 "foo1.17549435E-38” is changed to "foo1.1754944E-38” then the 
evaluation at that line succeeds but then the one at line 173 fails. Making a similar 
change on this line does not help. I suspect that this is due to a difference between the 
new code and that in jdk.internal.math.FloatingDecimal which is used by 
java.lang.AbstractStringBuilder and java.lang.invoke.StringConcatFactory but I’ve not 
actually investigated this as yet. I had actually wondered whether some or all of the 
FloatingDecimal code could be superseded by the updated logic.



"1.1754944E-38" is shorter than "1.17549435E-38" and can still recover 
Float.MIN_NORMAL. That's why the new code returns the shorter variant.


Why this works when the replacement is made in line 172 but not in line 
173 is really counter-intuitive. The only superficial difference is that 
FLOAT_MIN_NORM_1 is declared final while FLOAT_MIN_NORM_2 is not, 
although both are initialized with the same value.


The black-box conclusion is that it seems that string concatenation 
performed at compile time and at runtime manages to produce different 
results when given the same values. This is of course a problem outside 
the scope of the Double/Float conversions. As you point out, a more 
white-box analysis might reveal the culprit in the deadly embrace 
between string building and FloatingDecimal.


Unfortunately, I won't have time to investigate this interesting issue 
further before the weekend.



Greetings
Raffaello





Brian

[1] http://cr.openjdk.java.net/~bpb/4511638/webrev.00/
[2] java/lang/String/concat/ImplicitStringConcatBoundaries.java

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi Andrew,

In principle I agree with you.

However, in this case the maths underlying the algorithm to select the 
decimal are too involved to explain in comment form. I'm in the course 
of preparing a paper that explains the idea and the details. Then it 
should be easier to make sense out of the code.


Since to my knowledge the algorithm is novel, it will require some time 
for me to translate in paper form.


Other observations are interspersed with yours below.



On 2018-09-27 11:03, Andrew Haley wrote:

On 09/26/2018 06:39 PM, raffaello.giulie...@gmail.com wrote:


The submitted code contains both the changes to the current
implementation and extensive jtreg tests.

While I've struggled to keep the code within the 80 chars/line limit,
mercurial still generates longer lines. Thus, to avoid possible problems
with the email systems, the code is submitted both inline and as an
attachment. Hope at least one copy makes its way without errors.


Overall, the commenting is much too light. There are many places
where I think I know what you're doing but you don't explain it.

Here, for example:


+
+// pow5 = pow51*2^63 + pow50
+long pow51 = ceilPow5dHigh(-k);
+long pow50 = ceilPow5dLow(-k);
+
+// p = p2*2^126 + p1*2^63 + p0 and p = pow5 * cb
+long x0 = pow50 * cb;
+long x1 = multiplyHigh(pow50, cb);
+long y0 = pow51 * cb;
+long y1 = multiplyHigh(pow51, cb);
+long z = (x1 << 1 | x0 >>> 63) + (y0 & MASK_63);
+long p0 = x0 & MASK_63;
+long p1 = z & MASK_63;
+long p2 = (y1 << 1 | y0 >>> 63) + (z >>> 63);
+long vn = p2 << 1 + ord2alpha | p1 >>> 62 - ord2alpha;
+if ((p1 & mask) != 0 || p0 >= threshold) {
+vn |= 1;
+}


... etc. I think I can figure out what you're doing, but you could
explain it.


> If you write the comments now while the code is still fresh in your
> mind it'll be easy.
>

There are some succinct comments here that explain the expected results. 
I'm not the kind of programmer that comments every line since here the 
mechanics is simple enough to follow in Java directly. A good 
explanation would either be mathematical, which requires better 
typography than US-ASCII, or some explanatory drawings.


What the semantics of vn, vnl and vnr are will be explained in the 
future paper mentioned above.





+private static final long[] ceilPow5d = {
+/* -292 */ 0x7FBB_D8FE_5F5E_6E27L, 0x497A_3A27_04EE_C3DFL,
+/* -291 */ 0x4FD5_679E_FB9B_04D8L, 0x5DEC_6458_6315_3A6CL,
+/* -290 */ 0x63CA_C186_BA81_C60EL, 0x7567_7D6E_7BDA_8906L,
+/* -289 */ 0x7CBD_71E8_6922_3792L, 0x52C1_5CCA_1AD1_2B48L,


What exactly is this table anyway?  How was it generated?  Please say.



The comments of the accessor methods that make use of this private table 
implicitly explain its semantics as well. I will add a comment to the 
field that refers to the comments in the methods.



How the table was generated and thoroughly verified is currently not 
part of my contribution to OpenJDK, not because it is something secret 
or complex but because I think it is irrelevant here.


Besides, where would the generator/verifier code be placed in the 
codebase? It would be completely detached from, and unrelated to, 
everything else. But maybe there is already some mechanism in place for 
similar "bootstrapping" code in the OpenJDK. Then I would like to know 
to consider adding the generator there.




There are many more places in the code. What you've done is nice, but
it could be exemplary.



As said, this will be part of a separate paper. Hope this helps for the 
moment.



Thanks for your interest
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[raffaello . giulietti]

Hi Andrew,


On 2018-09-27 14:57, Andrew Dinn wrote:
> On 27/09/18 13:23, Raffaello Giulietti wrote:
>   . . .
>> The comments of the accessor methods that make use of this private table
>> implicitly explain its semantics as well. I will add a comment to the
>> field that refers to the comments in the methods.
>>
>>
>> How the table was generated and thoroughly verified is currently not
>> part of my contribution to OpenJDK, not because it is something secret
>> or complex but because I think it is irrelevant here.
> 
> I suspect it would be far from irrelevant to someone faced with
> debugging and fixing any future problem found in the code. Of course,
> you may well still be around to fix it but we don't know that for sure.
> 
>> Besides, where would the generator/verifier code be placed in the
>> codebase? It would be completely detached from, and unrelated to,
>> everything else. But maybe there is already some mechanism in place for
>> similar "bootstrapping" code in the OpenJDK. Then I would like to know
>> to consider adding the generator there.
> 
> We don't absolutely need generator/verifier code (although the latter
> might be helpful). The problem we face is what to do if a bug is found.
> Could it simply be a table entry that is wrong or is there a detail of
> the algorithm that has been mis-encoded? How would we tell?
> > If we have an explanation of /how/ the provided values were derived and
> /why/ so derived then that would allow any such bugs to be resolved much
> more easily. Omitting that background risks turning this into a one-shot
> code drop rather than a very welcome contribution.
> 

As mentioned in my previous post, the explanation of *what* the table
represents is implicitly and rigorously explained in the comments of the
methods that make use of it. That should make it easy to verify that the
table is sound.

As of the *how*, the generator is based on BigInteger arithmetic and
both generates and verifies the results. What I can do is to add the
code in the MathUtils class itself. It would never be run except for
diagnosis. But then one has to trust BigInteger.

The *why* will be part of the paper.



>>> There are many more places in the code. What you've done is nice, but
>>> it could be exemplary.
>>>
>>
>> As said, this will be part of a separate paper. Hope this helps for the
>> moment.
> Sure, a reference to a published doc would be great - assuming it makes
> it clear how the quite code is derived from the maths/algorithm it
> details. I'd really much prefer to have that doc before accepting the
> code just in order to be sure that there is no gap between theory and
> execution that some judicious commenting might close. Having recently
> reviewed some math code for log, trig and power functions I am well
> aware how details of specific coding operations are not always clearly
> identifiable from an abstract mathematical treatment. Even when they can
> be derived a few comments in the code often help avoid any resort to a
> pencil and thick pad of graph paper.
> 
> When you do post a link to the paper I'll be willing to check it and
> hope that I will, if needed, be able to ask you for advice to help
> clarify any such gap.
> 

The paper will explain the theory and the details.

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[raffaello . giulietti]

Hi Andrew,

On 2018-09-27 15:28, Andrew Haley wrote:
> On 09/27/2018 01:23 PM, Raffaello Giulietti wrote:
> 
>> In principle I agree with you.
>>
>> However, in this case the maths underlying the algorithm to select the 
>> decimal are too involved to explain in comment form. I'm in the course 
>> of preparing a paper that explains the idea and the details. Then it 
>> should be easier to make sense out of the code.
> 
> In which case, the code must contain pointers to the relevant parts of
> the paper.
> 

Sure.


>> Since to my knowledge the algorithm is novel, it will require some time 
>> for me to translate in paper form.
> 
> Sure, but how do you expect anyone to review your code without the
> necessary explanation? Do you think that people should approve your
> patch without that paper?
> 

I've no idea on how OpenJDK code is reviewed, so I cannot honestly
answer your questions. What I can tell is that if I were a reviewer, I
wouldn't trust rather involved code like mine without an explanation,
exactly as you seem inclined to do.

On the other side, in April this year I submitted another quite fast and
supposedly correct algorithm on this mailing list and I referred to an
accompanying paper by myself that gives full explanations on that
variant. Except for a couple of persons in private, nobody cared to send
me any observation or comment, neither on the code nor on the paper.

The present algorithm is superior. I have the theory in notes, in my
head, on napkins, on paper sheets all over my desk and floors. But
rather than spending time on the paper itself, like I did almost in vain
for the April variant, I preferred investing it in coding, for several
reasons:
* Only code executes, not a paper.
* Only code gives results that can be compared against.
* Only code can give indications on performance enhancements.
* Only code is interesting to be submitted to the OpenJDK.
* Having a paper without having tried the ideas in code is half the fun
and half as useful.

It's only a matter of priorities and of limited time, my spare time.
That said, now that I have some feedback and that the code has been
exercised at least some 400 billions times on doubles without errors and
on all 2^32 floats without errors, I now know that there is some
interest and that it is worth pushing energies in the paper.



>> There are some succinct comments here that explain the expected
>> results.  I'm not the kind of programmer that comments every line
>> since here the mechanics is simple enough to follow in Java
>> directly. A good explanation would either be mathematical, which
>> requires better typography than US-ASCII, or some explanatory
>> drawings.
> 
> Sure, these can be offline somewhere. The difficulty is always the
> mapping from the mathematics onto the Java code, and this is what must
> be explained in the comments.
> 

... or in the paper.


Greetings
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On 2018-09-27 16:55, Andrew Dinn wrote:

Hi Raffaello,

On 27/09/18 15:20, raffaello.giulie...@gmail.com wrote:

Hi Andrew,
On the other side, in April this year I submitted another quite fast and
supposedly correct algorithm on this mailing list and I referred to an
accompanying paper by myself that gives full explanations on that
variant. Except for a couple of persons in private, nobody cared to send
me any observation or comment, neither on the code nor on the paper.


I'm sorry I didn't see that post. I would have been very happy to review
the paper as well as the code. Unfortunately, none of us have time to
catch everything and we certainly don't always see every contribution.



I understand that most people are busy, so I was not really surprised 
not to get feedback on a rather tiny issue in the overall huge codebase.





The present algorithm is superior. I have the theory in notes, in my
head, on napkins, on paper sheets all over my desk and floors. But
rather than spending time on the paper itself, like I did almost in vain
for the April variant, I preferred investing it in coding, for several
reasons:
* Only code executes, not a paper.
* Only code gives results that can be compared against.
* Only code can give indications on performance enhancements.
* Only code is interesting to be submitted to the OpenJDK.
* Having a paper without having tried the ideas in code is half the fun
and half as useful.


I think this only presents one side of the argument here. For code of
anything but the most basic complexity. Assuming that by paper you mean
anything that goes beyond executable statements, including comments,
list discussions and reviews like this one, design notes and documents,
specifications et al



My point is about priorities and the past experience with almost zero 
feedback on the former implementation, not that a paper isn't due or 
useless. On the contrary, I'm the first that would not trust my own code 
without an explanation.





Only a paper tells you what an executing piece of code is actually doing

Only paper tells you what the results produced by that code need to be
compared against to determine correctness, accuracy, etc

Only paper can tell you whether achieved performance is worse than or
better than can be expected (or where in between it lies)

Only paper can explain what OpenJDK is supposed to be doing, why and how
the specific elements of the implementation achieve that what/why i.e.
withouth that audit trail OpenJDK will be dead in the water in no time
at all

Having code without the paper to tell you what ideas it implements is no
fun and n use at all.



Why "no use at all"? That's unfair.

It might not be fun currently, but it is quite useful anyway in its 
present form, having produced, as of today, some 400 billions correct 
results in 1/13 of the time.





I think that last one exemplifies a key asymmetry that always needs to
be borne in mind. If your last contribution did not get any signifcant
review on this or some other list then I think we really messed up.



Except where noted above, I agree with these observations. Currently, 
however, I'm in the lucky position to have both the explanation and the 
code, so they don't apply for myself in this particular case.


I'm thinking on how to present the ideas in some sketchy form, just to 
share the fun and mathematically convince the inclined ones, before the 
paper is ready.




Greetings
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi Ulf,


On 2018-09-27 16:40, Ulf Adams wrote:

Hi Raffaello,

I am the author of a recent publication on double to string conversion 
[1] - the Ryu algorithm. I've been aware of the problems with the Jdk 
for several years, and am very much looking forward to improvements in 
correctness and performance in this area.




What a coincidence! I'm happy to hear that the quest for better 
floating->string conversions has not stopped. Tomorrow I'll download 
your paper and have a look at it during the weekend.




I have done some testing against my Java implementation of the Ryu 
algorithm described in the linked paper. Interestingly, I've found a few 
cases where they output different results. In particular:

1.0E-323 is printed as 9.9E-324
1.0E-322 is printed as 9.9E-323


If Ryu also produces 1 digit long outputs, then your results above are 
correct. But then Ryu should also output 5.0E-324 rather than 4.9E-324, 
for example.
Even better, it should output 5E-324, 1E-323 and 1E-322 because adding 
the .0 part might confuse a human reader to believe that 2 digits are 
really needed. But then 4.9E-324, 9.9E-324 and 9.9E-323 are closer to 
the double.


2 digits are for backward compatibility with the existing spec which 
requires at least one digit to the right of the decimal point.





It's likely that there are more such cases - I only ran a sample of 
double-precision numbers. Arguably, 9.9 is the correctly rounded 2-digit 
output and Ryu is incorrect here. That's what you get when you have a 
special case for Java without a correctness proof. :-(


In terms of performance, this algorithm performs almost exactly the same 
as my Java implementation of Ryu, although I'd like to point out that my 
C implementation of Ryu is quite a bit faster (though note that it 
generates different output, in particular, it only outputs a single 
digit of precision in the above cases, rather than two), and I didn't 
backport all the performance improvements from the Java version, yet. It 
looks like this is not coincidence - as far as I can see so far, it's 
algorithmically very similar, although it manages to avoid the loop I'm 
using in Ryu to find the shortest representation.


I have a few comments:

      *      It rounds to {@code v} according to the usual 
round-to-closest

      *     rule of IEEE 754 floating-point arithmetic.
- Since you're spelling out the rounding rules just below, this is 
duplicated, and by itself, it's unclear since it doesn't specify the 
specific sub-type (round half even).




I tried to save as much of the original spec wording as possible. 
Perhaps it isn't worthwhile.




- Naming: I'd strongly suggest to use variable names that relate to 
what's stored, e.g., m for mantissa, e for exponent, etc.




I currently prefer to be consistent with a forthcoming paper of mine on 
the subject. But thanks for the suggestion.




- What's not clear to me is how the algorithm determines how many digits 
to print.




You'll have to wait for the paper.


- Also, it might be nicer to move the long multiplications to a helper 
method - at least from a short look, it looks like the computations of 
vn, vnl, and vnr are identical.




I tried several variants: the current one seems to be the faster with 
the current optimizations of C2. Some day I'll also try with Graal.




- I looked through the spec, and it looks like all cases are 
well-defined. Yay!


I will need some more time to do a more thorough review of the code and 
more testing for differences. Unfortunately, I'm also traveling the next 
two weeks, so this might take a bit of time.




I thank you in advance for your willingness to review the code but my 
understanding is that only the officially appointed reviewers can 
approve OpenJDK contributions, which is of course a good policy. 
Besides, as two Andrews engineers from RedHat correctly observe, 
understanding the rationale of the code without the planned accompanying 
paper is hard.




I'm not a contributor to the Jdk, and this isn't my full-time job. I was 
lurking here because I was going to send a patch for the double to 
string conversion code myself (based on Ryu).




All my efforts on this projects are done in my unpaid spare time, too.



Thanks,

-- Ulf


> [1] https://dl.acm.org/citation.cfm?id=3192369
> [2] https://github.com/google/double-conversion
> [3] https://en.wikipedia.org/wiki/Rounding
>


Thank you
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On 2018-09-27 17:53, Andrew Dinn wrote:

On 27/09/18 16:37, Raffaello Giulietti wrote:
   . . .

I thank you in advance for your willingness to review the code but my
understanding is that only the officially appointed reviewers can
approve OpenJDK contributions, which is of course a good policy.
Besides, as two Andrews engineers from RedHat correctly observe,
understanding the rationale of the code without the planned accompanying
paper is hard.


Oh no, let me stop you right there! Anyone competent can offer a review
(well incompetent people can too but let's assume they get ignored :-).
Indeed, an expert critique is always very welcome and reviewers normally
get credited even if they have no official status. The official
reviewers are needed for a final say so. No one sensible is going to
reject clear and clearly justified advice.



An excellent policy, indeed!




I'm not a contributor to the Jdk, and this isn't my full-time job. I
was lurking here because I was going to send a patch for the double to
string conversion code myself (based on Ryu).



All my efforts on this projects are done in my unpaid spare time, too.

Which is very much appreciated, thank you.



Thank all of you for your interest in this issue.


Greetings
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[raffaello . giulietti]

Hi,

On 2018-10-02 20:17, Martin Buchholz wrote:
> Raffaello and Ulf should talk.

I work on this project in my spare time, which is limited and not
entirely devoted to programming for the OpenJDK, of course ;-)

Starting this weekend, I'll try to recollect my notes and organize them
in paper or tutorial form.

I cannot speak for Ulf, of course, but my understanding is that his
porting to Java is a spare-time project, too.



> It seems like Ulf's ryu project is trying to solve the same problem.
> ryu is also still being worked on, but there is already a published
> paper and ryu is being adopted by various core libraries.
> https://github.com/ulfjack/ryu
> https://dl.acm.org/citation.cfm?id=3192369
> Ulf, if you haven't already signed an Oracle Contributor Agreement for
> openjdk, you should do so.
> (Who knew printing floating point numbers could be so hard?)
> 

Well, printing floating point numbers is simple if there are no
performance concerns. Abstractly, it is only a matter of calculations
with rational numbers. It becomes harder when efficiency becomes part of
the goal.

The quest started about 30 years ago with a seminal paper of Steele &
White and seemed settled with an implementation by Gay around 1991.
Since then, other interesting approaches have seen light with the goal
of even better performance.


Greetings
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[raffaello . giulietti]

On 2018-10-03 11:12, Andrew Dinn wrote:
> On 02/10/18 19:17, Martin Buchholz wrote:
>> Raffaello and Ulf should talk.
>> It seems like Ulf's ryu project is trying to solve the same problem.
>> ryu is also still being worked on, but there is already a published paper
>> and ryu is being adopted by various core libraries.
>> https://github.com/ulfjack/ryu
>> https://dl.acm.org/citation.cfm?id=3192369
>> Ulf, if you haven't already signed an Oracle Contributor Agreement for
>> openjdk, you should do so.
>> (Who knew printing floating point numbers could be so hard?)
> 
> Well, Guy Steele and Jon L White, for starters (quite literally) [1]
> 

In my eyes, the clarity of that paper is almost mythical.
I wish I could follow their teaching myself.





> [1] https://dl.acm.org/citation.cfm?id=93559
> 
> regards,
> 
> 
> Andrew Dinn
> ---
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
> 

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[raffaello . giulietti]

On 2018-10-04 15:28, Ulf Adams wrote:
> On Thu, Sep 27, 2018 at 5:37 PM Raffaello Giulietti
> mailto:raffaello.giulie...@gmail.com>>
> wrote:
> 
> Hi Ulf,
> 
> 
> On 2018-09-27 16:40, Ulf Adams wrote:
> > Hi Raffaello,
> >
> > I am the author of a recent publication on double to string
> conversion
> > [1] - the Ryu algorithm. I've been aware of the problems with the Jdk
> > for several years, and am very much looking forward to
> improvements in
> > correctness and performance in this area.
> >
> 
> What a coincidence! I'm happy to hear that the quest for better
> floating->string conversions has not stopped. Tomorrow I'll download
> your paper and have a look at it during the weekend.
> 
> 
> Have you had a chance to take a look?
> 
> (I'm traveling for the next ~10 days and at a conference, so don't
> expect too much from me during that time.)
>  
> 

I had a cursory reading but couldn't dig deeper for now. If nothing
unexpected happens, I should be able to study your paper during the weekend.



> 
> 
> 
> > I have done some testing against my Java implementation of the Ryu
> > algorithm described in the linked paper. Interestingly, I've found
> a few
> > cases where they output different results. In particular:
> > 1.0E-323 is printed as 9.9E-324
> > 1.0E-322 is printed as 9.9E-323
> 
> If Ryu also produces 1 digit long outputs, then your results above are
> correct. But then Ryu should also output 5.0E-324 rather than 4.9E-324,
> for example.
> Even better, it should output 5E-324, 1E-323 and 1E-322 because adding
> the .0 part might confuse a human reader to believe that 2 digits are
> really needed. But then 4.9E-324, 9.9E-324 and 9.9E-323 are closer to
> the double.
> 
> 
> The C version produces 1 digit long outputs, and I was trying to follow
> the Java spec in the Java version, but the code to do so isn't quite
> right. Unfortunately, I haven't yet been able to fix it.
>  
> 
> 
> 2 digits are for backward compatibility with the existing spec which
> requires at least one digit to the right of the decimal point.
> 
> 
> >
> > It's likely that there are more such cases - I only ran a sample of
> > double-precision numbers. Arguably, 9.9 is the correctly rounded
> 2-digit
> > output and Ryu is incorrect here. That's what you get when you have a
> > special case for Java without a correctness proof. :-(
> >
> > In terms of performance, this algorithm performs almost exactly
> the same
> > as my Java implementation of Ryu, although I'd like to point out
> that my
> > C implementation of Ryu is quite a bit faster (though note that it
> > generates different output, in particular, it only outputs a single
> > digit of precision in the above cases, rather than two), and I didn't
> > backport all the performance improvements from the Java version,
> yet. It
> > looks like this is not coincidence - as far as I can see so far, it's
> > algorithmically very similar, although it manages to avoid the
> loop I'm
> > using in Ryu to find the shortest representation.
> >
> > I have a few comments:
> >
> >       *      It rounds to {@code v} according to the usual
> > round-to-closest
> >       *     rule of IEEE 754 floating-point arithmetic.
> > - Since you're spelling out the rounding rules just below, this is
> > duplicated, and by itself, it's unclear since it doesn't specify the
> > specific sub-type (round half even).
> >
> 
> I tried to save as much of the original spec wording as possible.
> Perhaps it isn't worthwhile.
> 
> 
> 
> > - Naming: I'd strongly suggest to use variable names that relate to
> > what's stored, e.g., m for mantissa, e for exponent, etc.
> >
> 
> I currently prefer to be consistent with a forthcoming paper of mine on
> the subject. But thanks for the suggestion.
> 
> 
> May I suggest that the paper also uses names that relate to what they're
> referring to? :-) Not that I've managed to do that very well myself...
> 

I tend to use short "mathematical" names that still evoke their
semantics. Will see if I manage to be consistent.



> 
> 
> 
> > - What's not clear to me is how the algorithm determines how many
> digits
> >

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On 2019-03-07 11:10, Brian Burkhalter wrote:


On Mar 7, 2019, at 10:04 AM, Andrew Haley > wrote:


On 3/6/19 7:31 PM,raffaello.giulie...@gmail.com 
wrote:

the latest version of the patch, replacing the one found at [1].
In the next days, my sponsor Brian Burkhalter will publish it as a 
webrev.


I still believe you'd be better off defining an unsigned multiplyHigh than
all that messing about with shifts and masks in rop().


There is in fact an open issue for unsigned multiplyHigh:

https://bugs.openjdk.java.net/browse/JDK-8188044

Brian




Hi Andrew,

a couple of weeks ago I tried to refactor the code assuming the 
existence of unsignedMultiplyHigh() (either as some future intrinsic or 
as a Java method) and a wider representations of g with either 127 or 
128 bits:

g = g1 2^64 + g0

with either
2^63 <= g1 < 2^64 (128 bits)

or
2^62 <= g1 < 2^63 (127 bits)

Unfortunately, the resulting code of rop() isn't any simpler. That's 
because then an intermediate sum can overflow the 64 bits of a long. As 
a consequence, there's need for more elaborate logic to determine

the carry and other slightly more complicated computations to assemble
the final result. All in all, the resulting code has more operations and 
looks longer.


I tried with four variants. In addition to the mults, which are needed 
anyway, the current code has 3 shifts, 3 adds, 2 bitwise logicals. As 
mentioned, I couldn't come up with a solution that would help reducing 
this count.


I would be glad to hear of better solutions and to write down a 
mathematical proof for the document.




In the meantime I got rid of the last division. There's no division at 
all in the whole algorithm.




Greetings
Raffaello

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[raffaello . giulietti]

On 2019-03-08 14:35, Andrew Haley wrote:
> Hi,
> 
> On 3/7/19 7:16 PM, Raffaello Giulietti wrote:
> 
>> a couple of weeks ago I tried to refactor the code assuming the 
>> existence of unsignedMultiplyHigh() (either as some future intrinsic or 
>> as a Java method) and a wider representations of g with either 127 or 
>> 128 bits:
>>  g = g1 2^64 + g0
>>
>> with either
>>  2^63 <= g1 < 2^64 (128 bits)
>>
>> or
>>  2^62 <= g1 < 2^63 (127 bits)
>>
>> Unfortunately, the resulting code of rop() isn't any simpler. That's 
>> because then an intermediate sum can overflow the 64 bits of a long. As 
>> a consequence, there's need for more elaborate logic to determine
>> the carry and other slightly more complicated computations to assemble
>> the final result. All in all, the resulting code has more operations and 
>> looks longer.
> 
> Ah, I see. I agree, we still don't quite have the full set of operations
> that we need in Java, in particular a nice way of doing an add with carry.
> 

Yes.


> Thank you for the explanation.
> 

You're welcome.


>> In the meantime I got rid of the last division. There's no division at 
>> all in the whole algorithm.
> 
> Excellent. This is looking very good indeed.
> 

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi,

On 18.04.19 21:29, Brian Burkhalter wrote:


On Apr 18, 2019, at 11:44 AM, Raffaello Giulietti 
<mailto:raffaello.giulie...@gmail.com>> wrote:


here's another revision of the patch. Its purpose is to overcome the 
test failure observed in [1]. To this end, the patch adds


FloatToDecimal.appendTo(float, Appendable) and
DoubleToDecimal.appendTo(double, Appendable)

static methods to help AbstractStringBuilder in using the corrected 
algorithm implemented in


FloatToDecimal.toString(float) and
DoubleToDecimal.toString(double), respectively.

The implementation has been jmh tested to make sure there are no 
performance regressions.


Thanks, Raffaello.

As there are now only less than two months left before Rampdown 1 for 
OpenJDK 13, I beg anybody interested in reviewing this patch to 
contact me for any question or clarification. Also, you might want to 
take a look at the CSR [2].


+1 on both counts.

As usual, Brian will make the patch available as webrev in the coming 
hours.


Please see

http://cr.openjdk.java.net/~bpb/4511638/webrev.03/

I wonder whether in the new AbstractStringBuilder.append() changes the 
constructs:

880 try {
881 FloatToDecimal.appendTo(f, this);
882 } catch (IOException ignored) {
883 assert false;
884 }
might be better as:
880 try {
881 FloatToDecimal.appendTo(f, this);
882 } catch (IOException cause) {
883 throw new RuntimeException(cause);
884 }
Comments appreciated.

Brian



The reason is that FloatToDecimal.appendTo() takes an Appendable as 
parameter. Appendable's append() methods throw a checked IOException. 
This cannot happen with AbstractStringBuilder (the this passed as 
argument), so the code catches the IOException just to please the 
compiler. The assert is there just in case, but control shall never pass 
there. Whatever conventions are in place to emphasize that control flow 
cannot reach some point, I'll adopt them. Let me know.


The same holds for DoubleToDecimal.appendTo().

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi,

the semantics of java.io.IOError is:
"Thrown when a serious I/O error has occurred"
which I guess is not appropriate here.


I think the best compromise is

880 try {
881 FloatToDecimal.appendTo(f, this);
882 } catch (IOException cause) {
883 throw new AssertionError("Code shall be unreachable", cause);
884 }

which is more explicit and doesn't rely on assertions being enabled.


Greetings
Raffaello



On 2019-04-19 19:21, Jason Mehrens wrote:

Maybe rename the catch variable from 'cause' or 'ignored' to 'unreachable' and 
then wrap java.io.IOException in java.io.IOError?

____
From: core-libs-dev  on behalf of Raffaello 
Giulietti 
Sent: Thursday, April 18, 2019 3:37 PM
To: Brian Burkhalter; core-libs-dev
Subject: Re: [PATCH] 4511638: Double.toString(double) sometimes produces 
incorrect results

Hi,

On 18.04.19 21:29, Brian Burkhalter wrote:



On Apr 18, 2019, at 11:44 AM, Raffaello Giulietti
mailto:raffaello.giulie...@gmail.com>> wrote:

here's another revision of the patch. Its purpose is to overcome the
test failure observed in [1]. To this end, the patch adds

FloatToDecimal.appendTo(float, Appendable) and
DoubleToDecimal.appendTo(double, Appendable)

static methods to help AbstractStringBuilder in using the corrected
algorithm implemented in

FloatToDecimal.toString(float) and
DoubleToDecimal.toString(double), respectively.

The implementation has been jmh tested to make sure there are no
performance regressions.


Thanks, Raffaello.


As there are now only less than two months left before Rampdown 1 for
OpenJDK 13, I beg anybody interested in reviewing this patch to
contact me for any question or clarification. Also, you might want to
take a look at the CSR [2].


+1 on both counts.


As usual, Brian will make the patch available as webrev in the coming
hours.


Please see

http://cr.openjdk.java.net/~bpb/4511638/webrev.03/

I wonder whether in the new AbstractStringBuilder.append() changes the
constructs:
880 try {
881 FloatToDecimal.appendTo(f, this);
882 } catch (IOException ignored) {
883 assert false;
884 }
might be better as:
880 try {
881 FloatToDecimal.appendTo(f, this);
882 } catch (IOException cause) {
883 throw new RuntimeException(cause);
884 }
Comments appreciated.

Brian



The reason is that FloatToDecimal.appendTo() takes an Appendable as
parameter. Appendable's append() methods throw a checked IOException.
This cannot happen with AbstractStringBuilder (the this passed as
argument), so the code catches the IOException just to please the
compiler. The assert is there just in case, but control shall never pass
there. Whatever conventions are in place to emphasize that control flow
cannot reach some point, I'll adopt them. Let me know.

The same holds for DoubleToDecimal.appendTo().

[PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi,

no new code this time, only a warm invitation to review the rather 
substantial patch submitted on 2019-04-18 [1] and the CSR [2].


I spent an insane amount of (unpaid) free time offering my contribution 
to solve a bug known since about 15 years. Thus, I think it is 
understandable that I'd like to see my efforts come to a successful 
conclusion, ideally for OpenJDK 13.



Greetings
Raffaello


P.S.
Some enjoyable properties of the novel algorithm:
* No objects are instantiated, except, of course, the resulting String.
* Loop-free core algorithm.
* Only int and long arithmetic.
* No divisions at all.
* 16x speedup (jmh).
* Randomized, yet reproducible deep diving tests (jtreg).
* Clear, unambiguous spec.
* All floats have been tested to fully meet the spec.
* Fully documented in [3] or in comments.



[1] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html

[2] https://bugs.openjdk.java.net/browse/JDK-8202555
[3] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

Re: [PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi Anthony,

On 16/06/2019 19.17, Anthony Vanelverdinghe wrote:


Hi Raffaello

While I don't have feedback on the actual math, here's a few suggestions:

- there's some use of non-ASCII characters in the patch. I don't think 
this is common in the JDK's Java sources, so you might want to replace 
them with their Unicode escapes. The characters are: ≤ (\u2264), ∞ 
(\u221e), × (\u00d7), ≥ (\u2265), … (\u2026), ≠ (\u2260), ⌊(\u230a), 
⌋(\u230b), · (\u00b7), β (\u03b2)


I'm not sure what you mean here: there are indeed usages of HTML 
entities like ∞ that appear as math symbols in the rendered 
Javadoc but the sources are 100% US-ASCII in my IDE and should be so 
even in the patch.


If not, I'd be interested in the "coordinates" of the characters outside 
of the US-ASCII set.



- there are 2 invocations of a deprecated String constructor for 
performance reasons. I don't know how big the performance difference 
is, but I would suggest replacing these invocations with `new 
String(bytes, StandardCharsets.US_ASCII)` instead, and filing a bug 
for the performance difference with the deprecated constructor


The perf diff is measurable. The chosen variant is the fastest that I 
could come up with. As long as the deprecated constructor does not 
become "forRemoval = true", if at all, I wouldn't worry. Even then, 
there's plenty of time to switch.


Until then I can try to understand why the used constructor is faster 
and perhaps file a bug as you suggest.




- there are 2 occurrences of a typo "left-to-tight"


Yep, thanks for your eagle's eyes!


Other than that, I can only say this is an impressive piece of work, 
so I hope some official Reviewers will help you add your contribution 
to the JDK.


The review process has indeed begun some days ago. However, since 
there's is quite some maths to digest to understand the code, a full 
review could take 1 to 2 days of concentrated reading: not the average 
review, I guess.


Plus, there's a change in the specification, which must undergo a 
further review.



Thanks for your time!

Greetings
Raffaello




Kind regards

Anthony

--------
*From:* core-libs-dev  on 
behalf of Raffaello Giulietti 

*Sent:* Monday, May 6, 2019 2:08:48 PM
*To:* core-libs-dev
*Subject:* [PATCH] 4511638: Double.toString(double) sometimes produces 
incorrect results

Hi,

no new code this time, only a warm invitation to review the rather
substantial patch submitted on 2019-04-18 [1] and the CSR [2].

I spent an insane amount of (unpaid) free time offering my contribution
to solve a bug known since about 15 years. Thus, I think it is
understandable that I'd like to see my efforts come to a successful
conclusion, ideally for OpenJDK 13.


Greetings
Raffaello


P.S.
Some enjoyable properties of the novel algorithm:
* No objects are instantiated, except, of course, the resulting String.
* Loop-free core algorithm.
* Only int and long arithmetic.
* No divisions at all.
* 16x speedup (jmh).
* Randomized, yet reproducible deep diving tests (jtreg).
* Clear, unambiguous spec.
* All floats have been tested to fully meet the spec.
* Fully documented in [3] or in comments.



[1]
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html
[2] https://bugs.openjdk.java.net/browse/JDK-8202555
[3] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi folks,

as nothing has substantially moved towards approval of the patch [1] and 
the corresponding CSR [2], let's split the issue into its constituents.


The CSR is currently finalized and needs a further review for approval. 
This requires much less time than a review of the implementation [1].


The CSR has been carefully written to match the observed behavior of the 
current OpenJDK 12 as close as possible while aiming at being more clear 
and 100% rigorous. It can be approved independently of the underlying 
implementation.


So please review and approve the CSR [2], regardless of the proposed 
improved implementation [1], which can be tackled later.



Greetings
Raffaello



[1] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html

[2] https://bugs.openjdk.java.net/browse/JDK-8202555
[3] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

Are classes generated by LambdaMetafactory special?

[Raffaello Giulietti]

Hello,

experiment suggests that classes generated by 
java.lang.invoke.LambdaMetafactory are somewhat special:


(1) getName() on the class returns a string of the form
Xxx$$Lambda$nn/0xhhh
where Xxx is a fully qualified class name (with periods '.' as package 
separators), nn is a decimal integer and hhh is a hex integer. What's 
the role of the slash '/' in the name?


(2) An invocation of Class.forName() with that name fails.

(3) Invoking java.lang.instrument.Instrumentation.isModifiableClass() 
with that class as an argument returns false.


Is this intentional or is it a bug?


Greetings
Raffaello

Re: Are classes generated by LambdaMetafactory special?

[Raffaello Giulietti]

Thanks Rémi and Mandy.

I still don't get the full rationale on why lambda classes should be 
treated so specially but at least I now understand the current behavior.



Greetings
Raffaello




On 05/08/2019 23.34, Remi Forax wrote:

It is intentional and the implementation details are planned to change in the 
future
(there are already some patches in the valhalla/nestmates branch).

The slash in the name is because you can create several classes from the same 
bytecode by patching it at runtime,
the number after the slash is for each patching.

Unlike a classical class, the class is not stored by a classloader in order to 
be garbage collected sooner, hence you can not find it this Class.forName.

Intrumentation of a lambda proxy tends to fail because there is no API to get 
the patched data and because the class name is not resolvable (see above) so 
the easier solution was to mark those classes as non-modifiable. This may 
change in the future.

Rémi

- Mail original -

De: "raffaello giulietti" 
À: "core-libs-dev" 
Envoyé: Lundi 5 Août 2019 23:02:48
Objet: Are classes generated by LambdaMetafactory special?



Hello,

experiment suggests that classes generated by
java.lang.invoke.LambdaMetafactory are somewhat special:

(1) getName() on the class returns a string of the form
 Xxx$$Lambda$nn/0xhhh
where Xxx is a fully qualified class name (with periods '.' as package
separators), nn is a decimal integer and hhh is a hex integer. What's
the role of the slash '/' in the name?

(2) An invocation of Class.forName() with that name fails.

(3) Invoking java.lang.instrument.Instrumentation.isModifiableClass()
with that class as an argument returns false.

Is this intentional or is it a bug?


Greetings
Raffaello

Are classes generated by LambdaMetafactory special?

[Raffaello Giulietti]

Hi Mandy,

thanks for the deeper explanation: all this makes much sense.


Greetings
Raffaello


>One property of a lambda proxy class is to be in the same nest as the
>caller class as it's logically part of the caller class (as it may
>access its private member).  VM anonymous class is an interim
>implementation solution until we provide a way to define a dynamically
>generated class as a nestmate of an existing class.
>
>Second property of proxy classes is that proxy classes are not
>symbolically referenced by other class and they are sole implementation
>of some methods or some classes/interfaces.  Proxy classes can be
>defined as ordinary class while it has to ensure that the class name
>must be unique in the global VM namespace. There is overhead generating
>such a class to the system dictionary (not to mention loader
>constraints).  OTOH the name of a proxy class is only useful for
>troubleshooting but entirely not needed at runtime.
>
>Mandy
>
>On 8/5/19 2:51 PM, Raffaello Giulietti wrote:
>> Thanks Rémi and Mandy.
>>
>> I still don't get the full rationale on why lambda classes should be
>> treated so specially but at least I now understand the current behavior.
>>
>>
>> Greetings
>> Raffaello
>>
>>
>>
>>
>> On 05/08/2019 23.34, Remi Forax wrote:
>>> It is intentional and the implementation details are planned to
>>> change in the future
>>> (there are already some patches in the valhalla/nestmates branch).
>>>
>>> The slash in the name is because you can create several classes from
>>> the same bytecode by patching it at runtime,
>>> the number after the slash is for each patching.
>>>
>>> Unlike a classical class, the class is not stored by a classloader in
>>> order to be garbage collected sooner, hence you can not find it this
>>> Class.forName.
>>>
>>> Intrumentation of a lambda proxy tends to fail because there is no
>>> API to get the patched data and because the class name is not
>>> resolvable (see above) so the easier solution was to mark those
>>> classes as non-modifiable. This may change in the future.
>>>
>>> Rémi
>>>
>>> - Mail original -
>>>> De: "raffaello giulietti" 
>>>> À: "core-libs-dev" 
>>>> Envoyé: Lundi 5 Août 2019 23:02:48
>>>> Objet: Are classes generated by LambdaMetafactory special?
>>>
>>>> Hello,
>>>>
>>>> experiment suggests that classes generated by
>>>> java.lang.invoke.LambdaMetafactory are somewhat special:
>>>>
>>>> (1) getName() on the class returns a string of the form
>>>>  Xxx$$Lambda$nn/0xhhh
>>>> where Xxx is a fully qualified class name (with periods '.' as package
>>>> separators), nn is a decimal integer and hhh is a hex integer. What's
>>>> the role of the slash '/' in the name?
>>>>
>>>> (2) An invocation of Class.forName() with that name fails.
>>>>
>>>> (3) Invoking java.lang.instrument.Instrumentation.isModifiableClass()
>>>> with that class as an argument returns false.
>>>>
>>>> Is this intentional or is it a bug?
>>>>
>>>>
>>>> Greetings
>>>> Raffaello
>
>

GENTLE REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi,

just a gentle reminder for a RFR of [1] (see [5] for the original request).

An efficient implementation can be found on [2] and in Brian 
Burkhalter's webrev area [3]. This also requires a review. The full 
details are discussed in [4].



Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html

[3] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[4] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058
[5] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-July/061548.html

Why you want a better specification and a fast implementation of Double.toString(double)

[Raffaello Giulietti]

Hi,

I'd like this community to help me pushing a new spec and the 
accompanying implementation of Double.toString(double) and 
Float.toString(float) into the OpenJDK 14 release.


All material has already been submitted to this mailing list months ago 
[2] and passes tier 1 tests. Yet, there hasn't been much discussion or 
progress.


I understand that everybody is busy with many other big and small 
issues, duties and enhancements. But while my sponsor Brian Burkhalter 
started to look at it, he can't do everything alone, so I'm still 
looking for reviewers interested in the subject.


I would also be much interested in tactics, strategies and techniques to 
accelerate the review and approval process. Languishing here and waiting 
for good souls is not a nice position to be ;-)




Greetings
Raffaello



Q: Why a new implementation? What's wrong with the current one?

A: The current one tries to adhere to the rather non-mathematical 
current spec and does so in a costly and buggy way. Not that the bugs 
produce patently wrong results, but they are annoying anyway. They can 
all be categorized under the "too many digits" kind of bug.




Q: Isn't there a risk in replacing the current implementation with a new 
one?


A: Every change in the OpenJDK bears potential risks. On the other hand, 
the proposed implementation has been extensively tested in all aspects, 
without troubles. The risks are related more to reliance on current 
(mis)behavior than to unnoticed bugs in the new implementation.




Q: What are the cons of the new implementation?

A: Technically, there are none known to me. As alluded above, the only 
disadvantage is that the new outcomes are sometimes different, and then 
for the better (shorter outputs). Code that relies on the exact current 
behavior can possibly break.
For example, a test that expects Double.toString(2e23) to produce 
"1.9998E23", as currently done, rather than "2.0E23", as 
returned by the new implementation, will fail.




Q: Can you list some of the pros of the new implementation?

A: Two stand out:
(1) on the average, a conversion is about 15x faster (around 90 ns on a 
6 years old home-grade laptop)

(2) no temporary objects are ever allocated



Q: Why is there a need for another spec?

A: Because the current one is rather unclear in its main goal. In 
addition, it intermingles the selection of a decimal to represent the 
floating-point number with its formatting, which adds to the unclarity.




Q: What about the proposed spec.
A: It clearly separates the rather simple, mathematical and 100% 
unambiguous selection of the decimal from the formatting aspects.




Q: Which kind of help is needed?

A: You can:
(1) Take a look at the proposed spec [1] and send questions and comments 
to this mailing list.
(2) Take a look at the overall structure of the implementation [2] 
(webrev is in [3]) and discuss it here, even without understanding the 
underlying algorithms. The focus could be on quality, conventions, etc.
(3) Take a look at the accompanying paper [4] which presents all the 
nitty-gritty details and let me know if there's something unclear or wrong.
(4) You can try out the code. It is now about 6 months old, meant for 
and tested on OpenJDK 13. I guess it will work on OpenJDK 14 as well, 
but I didn't try out as of today.




Q: What is the investment in time?

A: The spec can be read and understood in about 10-15 minutes.
The paper needs 0.5-1.0 days, depending on your willingness to 
understand every detail. Once the paper has been worked out, however, 
the core of the implementation is straightforward to understand, say a 
couple of hours.




[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html

[3] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[4] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

Re: GENTLE REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Since nothing seems to have moved, here's the familiar monthly reminder ;-)

Greetings
Raffaello




On 2019-08-30 10:47, Raffaello Giulietti wrote:

Hi,

just a gentle reminder for a RFR of [1] (see [5] for the original request).

An efficient implementation can be found on [2] and in Brian 
Burkhalter's webrev area [3]. This also requires a review. The full 
details are discussed in [4].



Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html 


[3] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[4] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058
[5] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-July/061548.html

Re: GENTLE REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Yet another monthly reminder.
The December rampdown is approaching...

Greetings
Raffaello


On 2019-10-29 20:33, Raffaello Giulietti wrote:

Since nothing seems to have moved, here's the familiar monthly reminder ;-)

Greetings
Raffaello




On 2019-08-30 10:47, Raffaello Giulietti wrote:

Hi,

just a gentle reminder for a RFR of [1] (see [5] for the original 
request).


An efficient implementation can be found on [2] and in Brian 
Burkhalter's webrev area [3]. This also requires a review. The full 
details are discussed in [4].



Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059783.html 


[3] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[4] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058
[5] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-July/061548.html 

wavl based alternative to red-black TreeMap

[Raffaello Giulietti]

Hi core librarians,

did anybody already explore wavl trees as a drop-in replacement to the 
red-black based TreeMap?


If yes, I'm curious about performance comparisons.

If not, I would be glad to invest some time on it as they seem to be 
never worse than red-black trees and in many application (no deletions, 
only insertions and searching) even better.


No promises about delivery...


Greetings
Raffaello

wavl based alternative to red-black TreeMap

[Raffaello Giulietti]

Not exactly the newest data structure. Wavl trees are with us since 2009 ;-)

HashMap could benefit from wavl trees as well. Afaik, it uses red-black 
trees in presence of collisions, right?


R



Nice !
there is a wikipedia article about WAVL tree [1].

I did not know this new kind of balanced tree, worth the exploration i believe.

and TreeMap code really needs some love anyway, unlike ArrayList and HashMap, 
this code has not be updated since a long time.

Rémi
[1] https://en.wikipedia.org/wiki/WAVL_tree


- Mail original -

De: "raffaello giulietti" 
À: "core-libs-dev" 
Envoyé: Dimanche 1 Décembre 2019 18:00:19
Objet: wavl based alternative to red-black TreeMap



Hi core librarians,

did anybody already explore wavl trees as a drop-in replacement to the
red-black based TreeMap?

If yes, I'm curious about performance comparisons.

If not, I would be glad to invest some time on it as they seem to be
never worse than red-black trees and in many application (no deletions,
only insertions and searching) even better.

No promises about delivery...


Greetings
Raffaello

request for sponsor: wavl based alternative to red-black TreeMap

[Raffaello Giulietti]

Hello,

is anybody interested in sponsoring me on exploring a wavl tree 
implementation of TreeMap [1]?


For some reason related to gmail, I'm not able to get messages targeted 
to the mailing list alone, so please cc: me as well.



Greetings
Raffaello


(Here are some highlights of wavl trees:
* worst case height when built without deletions: ~1.4404 lg n
* worst case number of rotations for a deletion: 2

As a comparison, the same figures for red-black trees:
* worst case height when built without deletions: ~2 lg n
* worst case number of rotations for a deletion: 3

Other measures are like those of red-black trees or better.)



[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/063737.html

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

[Raffaello Giulietti]

Hello,

great job!

I think that the doc of MemoryAddress.copy() should be explicit about 
the direction of the copying, so it should either:


* explicitly specify a direction, e.g., lower-to-higher addresses

* or specify that in the case of an overlap the copying is smart enough 
to not destroy the src bytes before they have landed in dst


* or accept a negative third argument to encode a higher-to-lower 
addresses copying direction.



Greetings
Raffaello




Hi,
as part of the effort to upstream the changes related to JEP 370 
(foreign memory access API) [1], I'd like to ask for a code review for 
the corresponding core-libs and hotspot changes:


http://cr.openjdk.java.net/~mcimadamore/panama/8234049/

A javadoc for the memory access API is also available here:

http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc/jdk/incubator/foreign/package-summary.html

Note: the patch passes tier1, tier2 and tier3 testing (**)


Here is a brief summary of the changes in java.base and hotspot (the 
remaining new files are implementation classes and tests for the new API):


* ciField.cpp - this one is to trust final fields in the foreign memory 
access implementation (otherwise VM doesn't trust memory segment bounds)


* Modules.gmk - these changes are needed to require that the incubating 
module is loaded by the boot loader (otherwise the above changes are 
useless)


* library_call.cpp - this one is a JIT compiler change to treat 
Thread.currentThread() as a well-known constant - which helps a lot in 
the confinement checks (thanks Vlad!)


* various Buffer-related changes; these changes are needed because the 
memory access API allows a memory segment to be projected into a byte 
buffer, for interop reasons. As such, we need to insert a liveness check 
in the various get/put methods. Previously we had an implementation 
strategy where a BB was 'decorated' by a subclass called ScopedBuffer - 
but doing so required some changes to the BB API (e.g. making certain 
methods non-final, so that we could decorate them). Here I use an 
approach (which I have discussed with Alan) which doesn't require any 
public API  changes, but needs to add a 'segment' field in Buffer - and 
then have constructors which keep track of this extra parameter.


* FileChannel changes - these changes are required so that we can reuse 
the Unmapper class from the MemorySegment implementation, to 
deterministically deallocate a mapped memory segment. This should be a 
'straight' refactoring, no change in behavior should occur here. Please 
double check.


* VarHandles - this class now provides a factory to create memory access 
VarHandle - this is a bit tricky, since VarHandle cannot really be 
implemented outside java.base (e.g. VarForm is not public). So we do the 
usual trick where we define a bunch of proxy interfaces (see 
jdk/internal/access/foreign) have the classes in java.base refer to 
these - and then have the implementation classes of the memory access 
API implement these interfaces.


* JavaNIOAccess, JavaLangInvokeAccess - because of the above, we need to 
provide access to otherwise hidden functionalities - e.g. creating a new 
scoped buffer, or retrieving the properties of a memory access handle 
(e.g. offset, stride etc.), so that we can implement the memory access 
API in its own separate module


* GensrcVarHandles.gmk - these changes are needed to enable the 
generation of the new memory address var handle implementations; there's 
an helper class per carrier (e.g. VarHandleMemoryAddressAsBytes, ...). 
At runtime, when a memory access var handle is needed, we dynamically 
spin a new VH implementation which makes use of the right carrier. We 
need to spin because the VH can have a variable number of access 
coordinates (e.g. depending on the dimensions of the array to be 
accessed). But, under the hood, all the generated implementation will be 
using the same helper class.


* tests - we've tried to add fairly robust tests, often checking all 
possible permutations of carriers/dimensions etc. Because of that, the 
tests might not be the easiest to look at, but they have proven to be 
pretty effective at shaking out issues.


I think that covers the main aspects of the implementation and where it 
differs from vanilla JDK.


P.S.

In the CSR review [2], Joe raised a fair point - which is MemoryAddress 
has both:


offset(long) --> move address of given offset
offset() --> return the offset of this address in its owning segment

And this was considered suboptimal, given both methods use the same name 
but do something quite different (one is an accessor, another is a 
'wither'). one obvious option is to rename the first to 'withOffset'. 
But I think that would lead to verbose code (since that is a very common 
operation). Other options are to:


* rename offset(long) to move(long), advance(long), or something else
* drop offset() - but then add an overload of MemorySegment::asSlice 
which takes an address ins

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

[Raffaello Giulietti]

Hi,

MemoryLayouts exposes BITS_8_BE and BITS_8_LE.

Is there a reason to have both or is just love for symmetries (which I 
share)?



Greetings
Raffaello


> Hi,
> as part of the effort to upstream the changes related to JEP 370 
(foreign memory access API) [1], I'd like to ask for a code review for 
the corresponding core-libs and hotspot changes:

>
> http://cr.openjdk.java.net/~mcimadamore/panama/8234049/
>
> A javadoc for the memory access API is also available here:
>
> 
http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc/jdk/incubator/foreign/package-summary.html

>
> Note: the patch passes tier1, tier2 and tier3 testing (**)
>
>
> Here is a brief summary of the changes in java.base and hotspot (the 
remaining new files are implementation classes and tests for the new API):

>
> * ciField.cpp - this one is to trust final fields in the foreign 
memory access implementation (otherwise VM doesn't trust memory segment 
bounds)

>
> * Modules.gmk - these changes are needed to require that the 
incubating module is loaded by the boot loader (otherwise the above 
changes are useless)

>
> * library_call.cpp - this one is a JIT compiler change to treat 
Thread.currentThread() as a well-known constant - which helps a lot in 
the confinement checks (thanks Vlad!)

>
> * various Buffer-related changes; these changes are needed because 
the memory access API allows a memory segment to be projected into a 
byte buffer, for interop reasons. As such, we need to insert a liveness 
check in the various get/put methods. Previously we had an 
implementation strategy where a BB was 'decorated' by a subclass called 
ScopedBuffer - but doing so required some changes to the BB API (e.g. 
making certain methods non-final, so that we could decorate them). Here 
I use an approach (which I have discussed with Alan) which doesn't 
require any public API  changes, but needs to add a 'segment' field in 
Buffer - and then have constructors which keep track of this extra 
parameter.

>
> * FileChannel changes - these changes are required so that we can 
reuse the Unmapper class from the MemorySegment implementation, to 
deterministically deallocate a mapped memory segment. This should be a 
'straight' refactoring, no change in behavior should occur here. Please 
double check.

>
> * VarHandles - this class now provides a factory to create memory 
access VarHandle - this is a bit tricky, since VarHandle cannot really 
be implemented outside java.base (e.g. VarForm is not public). So we do 
the usual trick where we define a bunch of proxy interfaces (see 
jdk/internal/access/foreign) have the classes in java.base refer to 
these - and then have the implementation classes of the memory access 
API implement these interfaces.

>
> * JavaNIOAccess, JavaLangInvokeAccess - because of the above, we need 
to provide access to otherwise hidden functionalities - e.g. creating a 
new scoped buffer, or retrieving the properties of a memory access 
handle (e.g. offset, stride etc.), so that we can implement the memory 
access API in its own separate module

>
> * GensrcVarHandles.gmk - these changes are needed to enable the 
generation of the new memory address var handle implementations; there's 
an helper class per carrier (e.g. VarHandleMemoryAddressAsBytes, ...). 
At runtime, when a memory access var handle is needed, we dynamically 
spin a new VH implementation which makes use of the right carrier. We 
need to spin because the VH can have a variable number of access 
coordinates (e.g. depending on the dimensions of the array to be 
accessed). But, under the hood, all the generated implementation will be 
using the same helper class.

>
> * tests - we've tried to add fairly robust tests, often checking all 
possible permutations of carriers/dimensions etc. Because of that, the 
tests might not be the easiest to look at, but they have proven to be 
pretty effective at shaking out issues.

>
> I think that covers the main aspects of the implementation and where 
it differs from vanilla JDK.

>
> P.S.
>
> In the CSR review [2], Joe raised a fair point - which is 
MemoryAddress has both:

>
> offset(long) --> move address of given offset
> offset() --> return the offset of this address in its owning segment
>
> And this was considered suboptimal, given both methods use the same 
name but do something quite different (one is an accessor, another is a 
'wither'). one obvious option is to rename the first to 'withOffset'. 
But I think that would lead to verbose code (since that is a very common 
operation). Other options are to:

>
> * rename offset(long) to move(long), advance(long), or something else
> * drop offset() - but then add an overload of MemorySegment::asSlice 
which takes an address instead of a plain long offset

>
> I'll leave the choice to the reviewers
>
>
>
> Finally, I'd like to thank Mark, Brian, John, Alan, Paul, Vlad, 
Stuart, Roger, Joe and the Panama team for the feedback provided so far, 
which helped to get

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

[Raffaello Giulietti]

Hi,

will there be a

MemoryAddress.move(MemoryAddress src, MemoryAddress dst, long bytes)

method with POSIX memmove(3) semantics at some point?

That would be useful, e.g., to "open a hole" into an array by shifting 
existing elements towards higher indices (provided there's room).


MemoryAddress.copy(), with its lower-to-higher semantics, doesn't really 
help here, so without move() one would need to code an explicit loop for 
such a case, I guess. Not a big deal, just a little bit annoying.



Greetings
Raffaello



On 2019-12-07 00:51, Maurizio Cimadamore wrote:


On 06/12/2019 18:29, Raffaello Giulietti wrote:

Hello,

great job!

I think that the doc of MemoryAddress.copy() should be explicit about 
the direction of the copying, so it should either:


Thanks! -  I'll rectify the doc to specify lower-to-higher.

Maurizio



* explicitly specify a direction, e.g., lower-to-higher addresses

* or specify that in the case of an overlap the copying is smart 
enough to not destroy the src bytes before they have landed in dst


* or accept a negative third argument to encode a higher-to-lower 
addresses copying direction.



Greetings
Raffaello




Hi,
as part of the effort to upstream the changes related to JEP 370 
(foreign memory access API) [1], I'd like to ask for a code review 
for the corresponding core-libs and hotspot changes:


http://cr.openjdk.java.net/~mcimadamore/panama/8234049/

A javadoc for the memory access API is also available here:

http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc/jdk/incubator/foreign/package-summary.html 



Note: the patch passes tier1, tier2 and tier3 testing (**)


Here is a brief summary of the changes in java.base and hotspot (the 
remaining new files are implementation classes and tests for the new 
API):


* ciField.cpp - this one is to trust final fields in the foreign 
memory access implementation (otherwise VM doesn't trust memory 
segment bounds)


* Modules.gmk - these changes are needed to require that the 
incubating module is loaded by the boot loader (otherwise the above 
changes are useless)


* library_call.cpp - this one is a JIT compiler change to treat 
Thread.currentThread() as a well-known constant - which helps a lot 
in the confinement checks (thanks Vlad!)


* various Buffer-related changes; these changes are needed because 
the memory access API allows a memory segment to be projected into a 
byte buffer, for interop reasons. As such, we need to insert a 
liveness check in the various get/put methods. Previously we had an 
implementation strategy where a BB was 'decorated' by a subclass 
called ScopedBuffer - but doing so required some changes to the BB 
API (e.g. making certain methods non-final, so that we could decorate 
them). Here I use an approach (which I have discussed with Alan) 
which doesn't require any public API  changes, but needs to add a 
'segment' field in Buffer - and then have constructors which keep 
track of this extra parameter.


* FileChannel changes - these changes are required so that we can 
reuse the Unmapper class from the MemorySegment implementation, to 
deterministically deallocate a mapped memory segment. This should be 
a 'straight' refactoring, no change in behavior should occur here. 
Please double check.


* VarHandles - this class now provides a factory to create memory 
access VarHandle - this is a bit tricky, since VarHandle cannot 
really be implemented outside java.base (e.g. VarForm is not public). 
So we do the usual trick where we define a bunch of proxy interfaces 
(see jdk/internal/access/foreign) have the classes in java.base refer 
to these - and then have the implementation classes of the memory 
access API implement these interfaces.


* JavaNIOAccess, JavaLangInvokeAccess - because of the above, we need 
to provide access to otherwise hidden functionalities - e.g. creating 
a new scoped buffer, or retrieving the properties of a memory access 
handle (e.g. offset, stride etc.), so that we can implement the 
memory access API in its own separate module


* GensrcVarHandles.gmk - these changes are needed to enable the 
generation of the new memory address var handle implementations; 
there's an helper class per carrier (e.g. 
VarHandleMemoryAddressAsBytes, ...). At runtime, when a memory access 
var handle is needed, we dynamically spin a new VH implementation 
which makes use of the right carrier. We need to spin because the VH 
can have a variable number of access coordinates (e.g. depending on 
the dimensions of the array to be accessed). But, under the hood, all 
the generated implementation will be using the same helper class.


* tests - we've tried to add fairly robust tests, often checking all 
possible permutations of carriers/dimensions etc. Because of that, 
the tests might not be the easiest to look at, but they have proven 
to be pretty effective at shaking out issues.


I thi

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

[Raffaello Giulietti]

Fine, thanks!

Raffaello


On 2019-12-09 22:30, Maurizio Cimadamore wrote:

Hi again,
after some offline discussions with the hotspot team, it became clear 
that the recently added restriction on source/destination segments being 
disjoint is redundant and that Unsafe::copyMemory can cope with overlap. 
For now I'l keep the API as is, but I will file a followup issue to 
remove the restriction from the API - which, I think, will also address 
your issue around lack of memmove ability.


Thanks
Maurizio

On 09/12/2019 11:10, Maurizio Cimadamore wrote:

Hi Raffaello,
I think there is room to add more copy-related features in the future. 
One thing to note, however: the rationale behind having a 'copy' 
method is to expose a (safe) interface to the underlying 
Unsafe::copyMemory method - which is an hotspot intrinsic, hence 
understood and optimized by the JIT compiler. I'd say that, if we were 
to add another method like 'memmove' we'd have to work out the VM 
plumbing first - and then add the MemoryAddress API method. This has 
been the informal guiding principle for the operations exposed by this 
API.


That said - I'll keep a note of the suggestion, and add it to the list.

Cheers
Maurizio

On 09/12/2019 10:11, Raffaello Giulietti wrote:

Hi,

will there be a

MemoryAddress.move(MemoryAddress src, MemoryAddress dst, long bytes)

method with POSIX memmove(3) semantics at some point?

That would be useful, e.g., to "open a hole" into an array by 
shifting existing elements towards higher indices (provided there's 
room).


MemoryAddress.copy(), with its lower-to-higher semantics, doesn't 
really help here, so without move() one would need to code an 
explicit loop for such a case, I guess. Not a big deal, just a little 
bit annoying.



Greetings
Raffaello



On 2019-12-07 00:51, Maurizio Cimadamore wrote:


On 06/12/2019 18:29, Raffaello Giulietti wrote:

Hello,

great job!

I think that the doc of MemoryAddress.copy() should be explicit 
about the direction of the copying, so it should either:


Thanks! -  I'll rectify the doc to specify lower-to-higher.

Maurizio



* explicitly specify a direction, e.g., lower-to-higher addresses

* or specify that in the case of an overlap the copying is smart 
enough to not destroy the src bytes before they have landed in dst


* or accept a negative third argument to encode a higher-to-lower 
addresses copying direction.



Greetings
Raffaello




Hi,
as part of the effort to upstream the changes related to JEP 370 
(foreign memory access API) [1], I'd like to ask for a code review 
for the corresponding core-libs and hotspot changes:


http://cr.openjdk.java.net/~mcimadamore/panama/8234049/

A javadoc for the memory access API is also available here:

http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc/jdk/incubator/foreign/package-summary.html 



Note: the patch passes tier1, tier2 and tier3 testing (**)


Here is a brief summary of the changes in java.base and hotspot 
(the remaining new files are implementation classes and tests for 
the new API):


* ciField.cpp - this one is to trust final fields in the foreign 
memory access implementation (otherwise VM doesn't trust memory 
segment bounds)


* Modules.gmk - these changes are needed to require that the 
incubating module is loaded by the boot loader (otherwise the 
above changes are useless)


* library_call.cpp - this one is a JIT compiler change to treat 
Thread.currentThread() as a well-known constant - which helps a 
lot in the confinement checks (thanks Vlad!)


* various Buffer-related changes; these changes are needed because 
the memory access API allows a memory segment to be projected into 
a byte buffer, for interop reasons. As such, we need to insert a 
liveness check in the various get/put methods. Previously we had 
an implementation strategy where a BB was 'decorated' by a 
subclass called ScopedBuffer - but doing so required some changes 
to the BB API (e.g. making certain methods non-final, so that we 
could decorate them). Here I use an approach (which I have 
discussed with Alan) which doesn't require any public API changes, 
but needs to add a 'segment' field in Buffer - and then have 
constructors which keep track of this extra parameter.


* FileChannel changes - these changes are required so that we can 
reuse the Unmapper class from the MemorySegment implementation, to 
deterministically deallocate a mapped memory segment. This should 
be a 'straight' refactoring, no change in behavior should occur 
here. Please double check.


* VarHandles - this class now provides a factory to create memory 
access VarHandle - this is a bit tricky, since VarHandle cannot 
really be implemented outside java.base (e.g. VarForm is not 
public). So we do the usual trick where we define a bunch of proxy 
interfaces (see jdk/internal/access/foreign) have the classes in 
java.base 

GENTLE REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

The latest patches of [1] have been submitted more than 8 months ago [2].
I'm still looking for motivated reviewers.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/

Re: GENTLE REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On 2019-12-27 22:16, Andrew Haley wrote:
> On 12/27/19 11:43 AM, Raffaello Giulietti wrote:
>> The latest patches of [1] have been submitted more than 8 months ago 
[2].

>> I'm still looking for motivated reviewers.
>
> I am sorely aware that we have not performed well. All I can say in our
> defence is that we have looked at it, and a thorough review is hard,
> and there is significant risk touching this code. I know that Andrew
> Dinn would love to review it but he keeps getting dragged away to do
> other work. We have not forgotten you.
>


Hi Andrew & Andrew

thanks for your kind reply.

I'm aware that a review is harder than usual because of the size of the 
writing [1]. Of course, I can justify and explain every single line of 
the code if so requested. Most of it is based on details in the writing 
and is often a mere translation in Java.


To my knowledge, the algorithm is quite novel, so I cannot simply refer 
to existing, well-established and well-known work (except for some 
limited aspects on Adams' Ryu work [2], which are pretty coincidental).


Brain Burkhalter (Oracle) read the writing until section 8 included and 
stopped with section 9 which discusses the efficient implementation. I 
don't know if he read further in the meantime.


Except in answering specific questions about my contribution, I guess I 
can do very little to help those interested in finding the time to dive 
into the details. My estimate is that it takes between 0.5 and 1 day to 
understand the paper and less than a couple of hours to check that the 
code is a direct translation in Java. As usual, YMMV.



Greetings
Raffaello



[1] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058
[2] Adams, "Ryu: Fast Float-to-String Conversion", PLDI, 2018

GENTLE REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

This is the usual, familiar monthly reminder that the latest patches of 
[1] have been submitted more than 9 months ago [2] and still need a review.

Full background documentation is available at [3].


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[3] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hello,

here's the periodic monthly reminder that the latest patches of [1] have 
been submitted more than 10 months ago [2] and are waiting for a review.

Full background documentation is available at [3].


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] http://cr.openjdk.java.net/~bpb/4511638/webrev.03/
[3] https://drive.google.com/open?id=1KLtG_LaIbK9ETXI290zqCxvBW94dj058

Re: RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

No, the javadoc is identical, so there's no need to update the CSR.

R

On 2020-03-17 22:14, Brian Burkhalter wrote:

Raffaello,

Is there any change which would necessitate updating the CSR?

Thanks,

Brian

On Mar 17, 2020, at 2:13 PM, Brian Burkhalter 
mailto:brian.burkhal...@oracle.com>> wrote:


there's a new version of the documentation [1] for the attached 
patch to solve the long standing issues described in [2]. The CSR 
referred to in the subject is in [3].

REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hello,

here's the periodic monthly reminder that the latest patches to overcome 
[1] have been submitted [2] and are waiting for a review.

Full background documentation is available at [3].


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8202555
[2] http://cr.openjdk.java.net/~bpb/4511638/webrev.04/
[3] https://drive.google.com/open?id=1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN

17.7x speedup of Double.toString(double)

[Raffaello Giulietti]

Hello,

curious about Double.toString(double) performance?

Here are the results obtained by jmh 1.23 on a recent changeset [1]. The 
benchmark is over the full range of bitwise uniformly distributed random 
doubles.



# VM options: 
# Warmup: 5 iterations, 10 s each
# Measurement: 5 iterations, 10 s each
...
Benchmark Mode  Cnt Score Error  Units
DoubleToStringBenchmark.test  avgt   25  1251.658 ± 211.068  ns/op

showing quite a variability, probably attributable to intervening gc cycles.



The same setup after the integration of the code in [2] to replace 
Double.toString(double) with a modern, better specified and correct 
implementation (as witnessed by many trillions of fully conformant 
outcomes).



# VM options: 
# Warmup: 5 iterations, 10 s each
# Measurement: 5 iterations, 10 s each
...
Benchmark Mode  Cnt   Score   Error  Units
DoubleToStringBenchmark.test  avgt   25  70.712 ± 1.461  ns/op

showing both a speedup of 17.7x on my i7 based laptop of 2013 as well as 
predictable running times. The algorithm never generates temporary garbage.




I'm still looking for enthusiastic reviewers: they seem hard to find, 
despite monthly reminders... More in [3] and similarly titled posts for 
the past 12 months.



Greetings
Raffaello



[1] https://hg.openjdk.java.net/jdk/jdk/rev/90882ba9f488
[2] https://cr.openjdk.java.net/~bpb/4511638/webrev.04/
[3] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065476.html

Unexpected behaviour with larger Strings

[Raffaello Giulietti]

Hi,

I'm on Linux, but the explanation might be the same as the following one.

An easier way to obtain the same error on OpenJDK8 + HotSpot is to execute
byte[] b = new byte[Integer.MAX_VALUE];
which is exactly what happens behind the scenes in the UTF-8 case.

The encoder pessimistically assumes that each char will be encoded to at 
most 3 bytes. The expansion factor 3, however, is expressed as the float 
3.0f. This, in turn, is first converted to the double 3.0, multiplied by 
your length 1 << 30 and cast to int. As the product overflows the int 
range, the cast produces Integer.MAX_VALUE.


While Integer.MAX_VALUE should be considered a legal array size, I 
recall to have read somewhere that implementations are allowed to be a 
little bit more restrictive. Experimentally, the maximum size for a 
byte[] on OpenJDK8 + HotSpot / Linux is Integer.MAX_VALUE - 2. I guess 
it is the same on macOS.


Hope this helps.


Greetings
Raffaello

trivial correction to javadoc of j.l.i.MethodHandles.arrayElementVarHandle()

[Raffaello Giulietti]

Not sure if this requires an entry in the JBS...


Greetings
Raffaello




diff -r 70c2239696ae 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 16:12:53 2020 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 14:08:46 2020 +0200

@@ -3960,7 +3960,7 @@
  * {@code short}, {@code char}, {@code int}, {@code long},
  * {@code float}, or {@code double} then numeric atomic update 
access

  * modes are unsupported.
- * if the field type is anything other than {@code boolean},
+ * if the component type is anything other than {@code boolean},
  * {@code byte}, {@code short}, {@code char}, {@code int} or
  * {@code long} then bitwise atomic update access modes are
  * unsupported.

Re: trivial correction to javadoc of j.l.i.MethodHandles.arrayElementVarHandle()

[Raffaello Giulietti]

Hi Mandy,

OK, I'll file a record in the JBS. Thanks for sponsoring

Raffaello


On 2020-04-24 18:17, Mandy Chung wrote:

Yes any change requires a JBS issue.

I can sponsor this trivial fix.

Mandy

On 4/24/20 5:15 AM, Raffaello Giulietti wrote:

Not sure if this requires an entry in the JBS...


Greetings
Raffaello




diff -r 70c2239696ae 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 16:12:53 2020 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
Fri Apr 24 14:08:46 2020 +0200

@@ -3960,7 +3960,7 @@
  * {@code short}, {@code char}, {@code int}, {@code long},
  * {@code float}, or {@code double} then numeric atomic 
update access

  * modes are unsupported.
- * if the field type is anything other than {@code boolean},
+ * if the component type is anything other than {@code boolean},
  * {@code byte}, {@code short}, {@code char}, {@code int} or
  * {@code long} then bitwise atomic update access modes are
  * unsupported.

REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi,

after more than one year after first publication, the patches [1] and 
the CSR [2] are still awaiting a review...



Here's a breakdown of estimated times:
* Reading the CSR: 15-30 min. This can be done and approved independently.
* Checking that the implementation matches the documentation [3]: 1-2 hours.
* Reading the detailed documentation: 0.5-1.0 days.
* Overall review of the code, including the extensive tests: 0.5-1.0 days.


Greetings
Raffaello



[1] https://cr.openjdk.java.net/~bpb/4511638/webrev.04/
[2] https://bugs.openjdk.java.net/browse/JDK-8202555
[3] https://drive.google.com/open?id=1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN

Re: REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On 2020-04-30 15:29, Alan Bateman wrote:

On 30/04/2020 13:54, Raffaello Giulietti wrote:

Hi,

after more than one year after first publication, the patches [1] and 
the CSR [2] are still awaiting a review...



Here's a breakdown of estimated times:
* Reading the CSR: 15-30 min. This can be done and approved 
independently.
* Checking that the implementation matches the documentation [3]: 1-2 
hours.

* Reading the detailed documentation: 0.5-1.0 days.
* Overall review of the code, including the extensive tests: 0.5-1.0 
days.
This is impressive work. As I think was mentioned before, taking this on 
would require significant review effort and long term committent to 
maintain it. Has the paper been peer reviewed? Is the algorithm used by 
any other languages or run-times? I know Brian Burkhalter (and maybe 
Andrew Dinn too?) have put time into trying to read this but it's a 
significant effort and a lot more than a few days.


-Alan


Hi,

the writing has not undergone a formal peer review: this is why I care 
not to denote it as "paper", whenever I remember to do so.


I doubt a 27 pages long document written by an unknown individual not 
associated with any academic or industrial entity will ever be taken 
into consideration, not to say peer reviewed. That requires formally 
more "credible" writers supported by "authoritative" institutions, for 
some definition of "credible" and "authoritative".


The drafts and the first published version of the writing have been read 
over and over again by the late Dmitry Nadezhin, though. Dmitry was a 
JDK 8 Project Author associated with Oracle before his premature death 
last year. Apart from a couple of typos, he didn't complain about 
logical errors and was pleased with the outcome. Dmitry also contributed 
to an essential theorem and its proof, certified by ACL2 code.


Of course, I understand that having a published formal paper and the 
algorithm used somewhere else would add to the credibility and simplify 
acceptance in the OpenJDK. However, I guess that many novel technologies 
of all kind first appear in the OpenJDK than anywhere else (with 
associated risks probably higher than those of my contribution). 
Granted, they are much more strategical than my work to the good health, 
the progress and the future of this fantastic ecosystem.


Andrew Dinn expressed interested, indeed, but was (and probably still 
is) busy with many other duties.
Brian Burkhalter, my sponsor, read part of the writing some time ago and 
keeps the code on his webrev repo. He, too, is probably busy with other 
activities.


My estimate about the reviewing times assumes genuine enthusiasm and 
full immersion. With continuous interruptions to re-surface to tackle 
other duties, the times inflate for sure.


Seems that nobody has a chance or sufficient motivation to spend time 
with a review because the algorithm is novel and not used in the large. 
On the other hand, the algorithm is not used in the large exactly 
because it is novel and needs a review...
Adding to this, since more than 20 years there's a working, although 
expensive and not fully correct algorithm in the OpenJDK, so why bother 
to replace it, right?


I've no idea on how to overcome this stall. I feel kind of stuck, 
sitting here and waiting for somebody to step forward.



Greetings
Raffaello

Re: REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

If the float (binary32) case is of any guidance about the soundness of 
the algorithm and the validity of the implementation, *all* of the 2^32 
float bit patterns pass the extensive tests of the contribution in less 
than a couple of hours, as mentioned in past posts.


As a peace of mind, even simpler and quicker but not as extensive, 
anybody can check by very simple means that the String outcomes at least 
convert back to the original floats and are never longer than the 
current OpenJDK outcomes.


Exhaustive testing all 2^64 doubles is unreasonable on contemporary 
hardware, even in the cloud. Nevertheless, many trillions have been tested.



Greetings
Raffaello



On 2020-04-30 18:31, Raffaello Giulietti wrote:
> On 2020-04-30 15:29, Alan Bateman wrote:
>> On 30/04/2020 13:54, Raffaello Giulietti wrote:
>>> Hi,
>>>
>>> after more than one year after first publication, the patches [1] 
and the CSR [2] are still awaiting a review...

>>>
>>>
>>> Here's a breakdown of estimated times:
>>> * Reading the CSR: 15-30 min. This can be done and approved 
independently.
>>> * Checking that the implementation matches the documentation [3]: 
1-2 hours.

>>> * Reading the detailed documentation: 0.5-1.0 days.
>>> * Overall review of the code, including the extensive tests: 
0.5-1.0 days.
>> This is impressive work. As I think was mentioned before, taking 
this on would require significant review effort and long term committent 
to maintain it. Has the paper been peer reviewed? Is the algorithm used 
by any other languages or run-times? I know Brian Burkhalter (and maybe 
Andrew Dinn too?) have put time into trying to read this but it's a 
significant effort and a lot more than a few days.

>>
>> -Alan
>
> Hi,
>
> the writing has not undergone a formal peer review: this is why I 
care not to denote it as "paper", whenever I remember to do so.

>
> I doubt a 27 pages long document written by an unknown individual not 
associated with any academic or industrial entity will ever be taken 
into consideration, not to say peer reviewed. That requires formally 
more "credible" writers supported by "authoritative" institutions, for 
some definition of "credible" and "authoritative".

>
> The drafts and the first published version of the writing have been 
read over and over again by the late Dmitry Nadezhin, though. Dmitry was 
a JDK 8 Project Author associated with Oracle before his premature death 
last year. Apart from a couple of typos, he didn't complain about 
logical errors and was pleased with the outcome. Dmitry also contributed 
to an essential theorem and its proof, certified by ACL2 code.

>
> Of course, I understand that having a published formal paper and the 
algorithm used somewhere else would add to the credibility and simplify 
acceptance in the OpenJDK. However, I guess that many novel technologies 
of all kind first appear in the OpenJDK than anywhere else (with 
associated risks probably higher than those of my contribution). 
Granted, they are much more strategical than my work to the good health, 
the progress and the future of this fantastic ecosystem.

>
> Andrew Dinn expressed interested, indeed, but was (and probably still 
is) busy with many other duties.
> Brian Burkhalter, my sponsor, read part of the writing some time ago 
and keeps the code on his webrev repo. He, too, is probably busy with 
other activities.

>
> My estimate about the reviewing times assumes genuine enthusiasm and 
full immersion. With continuous interruptions to re-surface to tackle 
other duties, the times inflate for sure.

>
> Seems that nobody has a chance or sufficient motivation to spend time 
with a review because the algorithm is novel and not used in the large. 
On the other hand, the algorithm is not used in the large exactly 
because it is novel and needs a review...
> Adding to this, since more than 20 years there's a working, although 
expensive and not fully correct algorithm in the OpenJDK, so why bother 
to replace it, right?

>
> I've no idea on how to overcome this stall. I feel kind of stuck, 
sitting here and waiting for somebody to step forward.

>
>
> Greetings
> Raffaello
>

REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi,

afaik, review of [1] (webrev in [2]) didn't progress in the meantime.

I'm very well aware that a full review requires some time (see [3] for 
my estimates), depending on your familiarity in working out the maths in 
[4] (*no* numerics, *no* floating-point arithmetic, promised!).


Yet, the associated CSR [5] can be reviewed independently of the 
proposed algorithm and implementation. It is in "finalized" status since 
2019-06-03 and requires a last push to be "approved" (15-30 minutes of 
concentrated work). The CSR strives to be a rigorous spec that leaves no 
leeway for ambiguities in the outcomes. Would be nice if it could make 
it for Rampdown 1 of JDK 15 (2020-06-11).



Greetings
Raffaello



[1] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065291.html

[2] http://cr.openjdk.java.net/~bpb/4511638/webrev.04/
[3] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/066160.html

[4] https://drive.google.com/open?id=1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN
[5] https://bugs.openjdk.java.net/browse/JDK-8202555

contributing to JDK-8171407: Port fdlibm to Java, part 2

[Raffaello Giulietti]

Hi,

[1] enumerates 14 StrictMath native functions that still need a Java 
porting from the original C code.


As of changeset b84f122ea855 all of them are still native, so I wonder 
if it makes sense to contribute or if they have already been ported and 
on the launch pad for integration.


Let me know.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8171407

Clean-room implementation of Double::toString(double) and Float::toString(float)

[raffaello . giulietti]

Hi,

the topic I would like to work on is to solve the bugs described at
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4511638 about the
results produced by the current implementation of
Double::toString(double). I noticed that after all these years most of
the issues illustrated in the bug report still remain unsolved in the
latest OpenJDK.

To that end, I have worked on a clean-room implementation that:
* Could be used as a drop-in replacement for Double::toString(double)
* Solves *all* the anomalies still present in OpenJDK.
* Produces identical results otherwise.
* Has extensive mathematical documentation on the inner workings.
* Is way faster than the current OpenJDK implementation.
* Uses significantly less resources in most cases.
I guess this might sound rather presumptuous, but as far as I can tell
the claims are unbiased and reflect reality.

The new code also has a better specification than the current one, while
being mostly compatible. Indeed, the current specification leaves room
for interpretation and thus cannot ensure that an implementation
produces consistent and unique results from one release to the next. The
newer spec ensures a unique result.

If there is interest in the new implementation, I would be glad to
contribute my code to the OpenJDK. I've already signed the Oracle
Contributor Agreement a few days ago.

The code currently sits in its own module, is mostly polished,
thoroughly tested but has never been audited by people other than myself.

My target is to be able to integrate it in the JDK 11 LTS release, due
in late September 2018.



Greetings
Raffaello

Re: Clean-room implementation of Double::toString(double) and Float::toString(float)

[raffaello . giulietti]

Hi Brian,

On 2018-03-30 22:42, Brian Burkhalter wrote:
> Hello Raffaello,
> 
> On Mar 30, 2018, at 9:50 AM, raffaello.giulie...@gmail.com
>  wrote:
> 
>> the topic I would like to work on is to solve the bugs described at
>> https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4511638 about the
>> results produced by the current implementation of
>> Double::toString(double). I noticed that after all these years most of
>> the issues illustrated in the bug report still remain unsolved in the
>> latest OpenJDK.
> 
> The JBS [1] issue is [2]. It is currently owned by me.
> 

Yes, and the submission of the report was mine, far more than 10 years
ago. Guys, time passes too fast...



>> […]
>>
>> The new code also has a better specification than the current one, while
>> being mostly compatible. Indeed, the current specification leaves room
>> for interpretation and thus cannot ensure that an implementation
>> produces consistent and unique results from one release to the next. The
>> newer spec ensures a unique result.
> 
> Any specification change would need to go through the Compatibility and
> Specification Review process. [3]
> 

OK, as you will see, as soon as the code will be uploaded, the only
thing that formally affects output is the "1.0E23" versus "9.99E22"
issue. Everything else is worded in such a way to remain compatible but
is simply a little bit more rigorous.



>> If there is interest in the new implementation, I would be glad to
>> contribute my code to the OpenJDK.
> 
> Speaking for myself, any contribution in this area would be welcome.
> Work could proceed under the extant JBS bug ID.
> 

OK, let's to that.



>> I've already signed the Oracle Contributor Agreement a few days ago.
> 
> Processing the OCA should take at least two weeks [4].
> 

My wording was misleading: I already got the confirmation that my OCA
application has been accepted, so I'm formally ready to contribute.



>> The code currently sits in its own module, is mostly polished,
>> thoroughly tested but has never been audited by people other than myself.
>>
>> My target is to be able to integrate it in the JDK 11 LTS release, due
>> in late September 2018.
> 
> Per the JDK 11 schedule [5] there could well be sufficient time to run
> this submission through the review processes. I suggest, once your OCA
> has been processed, to proceed by posting your proposed changes for
> review on this mailing list. Note that in general attachments are
> scrubbed, so the patch would need either to be included inline or
> published as a webrev [6].
> 

OK, I'll take a look on how the mechanics works.

I'm usually on Windows. Are there technical issues with it as far as
Webrev is concerned? I mean, I could setup a Linux VM in VirtualBox if
this simplifies my life, but I'd prefer continuing my main work in Win.



> 
> [1] https://wiki.openjdk.java.net/display/general/JBS+Overview
> [2] https://bugs.openjdk.java.net/browse/JDK-4511638
> [3] https://wiki.openjdk.java.net/display/csr/Main
> [4] http://openjdk.java.net/contribute/
> [5] http://mail.openjdk.java.net/pipermail/jdk-dev/2018-March/000940.html
> [6] http://openjdk.java.net/guide/codeReview.html

Coding conventions for core-libs-dev

[raffaello . giulietti]

Hi,

are there any Java coding conventions to follow?

I found very outdated guidelines at
http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html
with ab archaic line limitation of 80 or even 70 chars/line.

Are there official, semi-official or suggested documents?



Greetings
Raffaello

Re: Coding conventions for core-libs-dev

[raffaello . giulietti]

On 2018-03-31 16:51, Mario Torre wrote:
> Yes, but please try to keep the 80 columns (as per the document Remi
> suggested, unless it kills readability). It seems archaic, but not so
> much when you review patches and read code on a mobile device. Lots of
> people also like to keep their monitors in portrait mode, which means
> less real estate for long lines, not talking about the fact that this
> limit forces you to write more concise code!
>
> Cheers,
> Mario

Portrait orientation might invite writing longer methods and choosing
shorter names, which is arguably even worse than long lines ;-)

Anyway, while 80 chars/line is not my first choice I'll or course adhere
to the guidelines mentioned by Rémi. BTW, does "80" include the ending LF?



Raffaello

> 
> 
> On Sat 31. Mar 2018 at 16:35, Remi Forax  <mailto:fo...@univ-mlv.fr>> wrote:
> 
> http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html
> is the closest you can find as an official coding convention for the
> OpenJDK.
> 
> Rémi
> 
> - Mail original -
> > De: "raffaello giulietti"  <mailto:raffaello.giulie...@gmail.com>>
> > À: "core-libs-dev"  <mailto:core-libs-dev@openjdk.java.net>>
> > Envoyé: Samedi 31 Mars 2018 16:24:53
> > Objet: Coding conventions for core-libs-dev
> 
> > Hi,
> >
> > are there any Java coding conventions to follow?
> >
> > I found very outdated guidelines at
> >
> 
> http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html
> > with ab archaic line limitation of 80 or even 70 chars/line.
> >
> > Are there official, semi-official or suggested documents?
> >
> >
> >
> > Greetings
> > Raffaello
> 

Re: Clean-room implementation of Double::toString(double) and Float::toString(float)

[Raffaello Giulietti]

Hi,

my code is now ready to be uploaded to the OpenJDK reps. Currently it 
resides on GitHub and is under the "GPLv2 + Classpath Exception" 
license, with myself as the copyright holder.


I asked Oracle about how the copyright notice should be adapted to meet 
the OCA requirements but, as of today, I got no answer.


Is there any official reference about the copyright notice on OpenJDK 
contributions?



Greetings
Raffaello




On 2018-03-31 00:17, Brian Burkhalter wrote:

Hi Raffaello,

On Mar 30, 2018, at 2:57 PM, raffaello.giulie...@gmail.com 
 wrote:



[…]

The new code also has a better specification than the current one, while
being mostly compatible. Indeed, the current specification leaves room
for interpretation and thus cannot ensure that an implementation
produces consistent and unique results from one release to the next. The
newer spec ensures a unique result.


Any specification change would need to go through the Compatibility and
Specification Review process. [3]



OK, as you will see, as soon as the code will be uploaded, the only
thing that formally affects output is the "1.0E23" versus "9.99E22"
issue. Everything else is worded in such a way to remain compatible but
is simply a little bit more rigorous.


Sounds good.


My wording was misleading: I already got the confirmation that my OCA
application has been accepted, so I'm formally ready to contribute.


That’s good as it gives more time.


Per the JDK 11 schedule [5] there could well be sufficient time to run
this submission through the review processes. I suggest, once your OCA
has been processed, to proceed by posting your proposed changes for
review on this mailing list. Note that in general attachments are
scrubbed, so the patch would need either to be included inline or
published as a webrev [6].



OK, I'll take a look on how the mechanics works.

I'm usually on Windows. Are there technical issues with it as far as
Webrev is concerned? I mean, I could setup a Linux VM in VirtualBox if
this simplifies my life, but I'd prefer continuing my main work in Win.


As seen in Jon’s posting there are some attachment types which will 
work. As to webrev, I think it should work on Windows at least in cygwin 
but I’ve not used it there myself. If it’s just a matter of creating a 
webrev I could do that on your behalf based on your supplied patch.


Thanks,

Brian

Re: Clean-room implementation of Double::toString(double) and Float::toString(float)

[Raffaello Giulietti]

Thanks Mark,

in your position as the Chief Architect, Java Platform Group at Oracle, 
I take yours as the most authoritative answer I've got so far.


Tomorrow I should find the time to finally upload the code.

Thanks to everybody who kindly replied to this thread.

Greetings
Raffaello



On 2018-04-17 00:52, mark.reinh...@oracle.com wrote:

2018/4/12 1:12:36 -0700, raffaello.giulie...@gmail.com:

my code is now ready to be uploaded to the OpenJDK reps. Currently it
resides on GitHub and is under the "GPLv2 + Classpath Exception"
license, with myself as the copyright holder.

I asked Oracle about how the copyright notice should be adapted to meet
the OCA requirements but, as of today, I got no answer.

Is there any official reference about the copyright notice on OpenJDK
contributions?


For library code, the template is in $JDK/make/templates/gpl-cp-header [1].
It begins:

   Copyright (c) %YEARS% Oracle and/or its affiliates. All rights reserved.
   DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

   This code is free software; you can redistribute it and/or modify it
   under the terms of the GNU General Public License version 2 only, as
   ...

You can just use the Oracle copyright line or, at your option, replace
it with your own.  In any case %YEARS% should be replaced with the year
in which the file was created, and if it was modified in any later year
then the creation year should be followed by the latest year in which
the file was modified, separated by a comma.

(We can better advise you on the details once we see the actual code.)

- Mark


[1] http://hg.openjdk.java.net/jdk/jdk/file/tip/make/templates/gpl-cp-header

uploading of new code

[raffaello . giulietti]

Hi,

in thread [1] I mention a clean room implementation of
Double.toString(double) and my willingness to contribute it to the
OpenJDK project to fix [2].

As of today, my code resides in a module that exports a package simply
named "math". The reason I do not alter the original OpenJDK
implementation is that I need it *along* my code to be able to compare
results. I developed the code in total ignorance of the OpenJDK
implementation, so to speak. This is not the end goal, of course, but
the safest way to review/test it without potentially breaking existing code.

As a consequence, I wonder how to best deliver my contribution to the
OpenJDK project in this phase. As far as I understand, webrev [3] is
better suited for *changes* to existing code, not specifically for *new*
code.

I also understand that attachments are not recommended. On the other
hand, pasting some thousands of Java lines in a post on the mailing list
isn't practical either, I guess.

The code currently consists of about 8 files for a total of about 3000 +
600 lines for code + tests.

So, how can I best proceed with this initial delivery?



Greetings
Raffaello


[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/052355.html
[2] https://bugs.openjdk.java.net/browse/JDK-4511638
[3] http://openjdk.java.net/guide/codeReview.html

Re: uploading of new code

[raffaello . giulietti]

On 2018-04-18 16:35, Aleksey Shipilev wrote:
> On 04/18/2018 03:58 PM, Andrew Haley wrote:
>> On 04/18/2018 02:55 PM, raffaello.giulie...@gmail.com wrote:
>>> So, how can I best proceed with this initial delivery?
>>
>> Once you have an OpenJDK id you can get an account on cr.openjdk.java.net.
>> That's where you should put your changes.
> 

@Andrew
I can't recall having got an OpenJDK id when I got my Oracle Contributor
Agreement confirmed. But I'll double check.



> ...and until that happens you can ask existing users to host the webrev for 
> you, probably your
> current sponsor. Brian Burkhalter seems to be your sponsor for this work?
> 

@Aleksey
Right, I suppose Brian is my sponsor.



Greetings
Raffaello

Re: uploading of new code

[Raffaello Giulietti]

On 2018-04-18 19:42, mark.reinh...@oracle.com wrote:

2018/4/18 10:27:36 -0700, e...@zusammenkunft.net:

Hello, I would put it on a standalone Git repo on one of the public
hosting sites lile.Github, especially for a first discussion
(especially good if added JMH comparisions). I would not expect a id
quickly/easily and a in-tree webrev could be created by sponsor for
final ok.


For IP clarity, no Oracle engineer will review code that is not posted,
in some form or another (e-mail, webrev, JIRA), directly to OpenJDK.

For an initial discussion a patch in e-mail is a fine way to start, even
for 3,000 lines of code.  Those who care can either read it directly or
else import it into a local repo for further evaluation.

- Mark



I already uploaded the code on a GitHub repo last week, just to have it 
published somewhere, but in fact didn't expect this move to be 
recognized as an official delivery to the OpenJDK project, hence never 
advertised the URL.


So I will deliver the exact same code as found on GitHub both to this 
mailing list and to Brian Burkhalter, who is the Oracle engineer in 
charge of [1].


Sorry guys for all these trivia.


Greetings
Raffaello


[1] https://bugs.openjdk.java.net/browse/JDK-4511638

Upload of drop-in replacement for Double:::toString(double)

[raffaello . giulietti]

library.  If
you modify this library, you may extend this exception to your version of
the library, but you are not obligated to do so.  If you do not wish to do
so, delete this exception statement from your version.

 module-info.java

/*
 * Copyright (c) 2018, Raffaello Giulietti. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 only, as
 * published by the Free Software Foundation.
 * This particular file is subject to the "Classpath" exception as provided
 * in the LICENSE file that accompanied this code.
 *
 * This code is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 * version 2 for more details (a copy is included in the LICENSE file that
 * accompanied this code).
 *
 * You should have received a copy of the GNU General Public License version
 * 2 along with this work; if not, write to the Free Software Foundation,
 * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
 */

module todec {
exports math;
}

 math.Natural.java

/*
 * Copyright (c) 2018, Raffaello Giulietti. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 only, as
 * published by the Free Software Foundation.
 * This particular file is subject to the "Classpath" exception as provided
 * in the LICENSE file that accompanied this code.
 *
 * This code is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 * version 2 for more details (a copy is included in the LICENSE file that
 * accompanied this code).
 *
 * You should have received a copy of the GNU General Public License version
 * 2 along with this work; if not, write to the Free Software Foundation,
 * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
 */

package math;

import java.math.BigInteger;

import static java.lang.Math.max;

/**
 * A minimal, limited implementation of non-negative large integers.
 *
 * All operations implemented here are needed in other parts of the
package,
 * while many are missing entirely because they are not needed.
 */
final class Natural {
private static final long I_SZ = Integer.SIZE;
private static final long I_MASK = (1L << I_SZ) - 1;
private static final int I_MSB = -1 << I_SZ - 1;
private static final long L_MSB = -1L << Long.SIZE - 1;

/*
A large natural is represented as a sequence of len B-ary digits,
that is, in base
B = 2^Integer.SIZE = 2^32
Its value is
d[0] + d[1]*B + d[2]*B^2 + ... + d[len-1]*B^(len-1)
where each B-ary digit d[i] is interpreted as unsigned int.
As usual, an empty sum has value 0, so if len = 0 then the value is 0.

The following invariants hold:
0 <= len <= d.length
either len = 0 or d[len-1] != 0
 */
private final int[] d;
private final int len;

private Natural(int[] d) {
int i = d.length;
while (--i >= 0 && d[i] == 0); // empty body intended
this.len = i + 1;
this.d = d;
}

/**
 * Returns a {@link Natural} with the same value as {@code v},
 * which is interpreted as an unsigned {@code long}.
 */
static Natural valueOf(long v) {
return new Natural(new int[]{(int) v, (int) (v >>> 32)});
}

/**
 * Returns a {@link Natural} with the value
 * {@code v} · 2{@code n}.
 * The value {@code v} is interpreted as an unsigned {@code long} and
 * {@code n} must be non-negative.
 */
static Natural valueOfShiftLeft(long v, int n) {
// I_SZ = 2^5
int q = n >> 5;
int r = n & 0x1f;
// length is q plus additional 2 for v and 1 for possible
overlapping
int[] rd = new int[q + 3];
rd[q] = (int) v << r;
rd[q + 1] = (int) (v >>> I_SZ - r);
// A safe shift by 64 - r, even when r = 0
rd[q + 2] = (int) (v >>> I_SZ >>> I_SZ - r);
return new Natural(rd);
}

/**
 * Returns -1, 0 or 1, depending on whether {@code this} is
 * <, = or > {@code y}, respectively.
 */
int compareTo(Natural y) {
if (len < y.len) {
return -1;
}
if (len > y.len) {
return 1;
}
int i = len;
while (--i >= 0 && d[i] == y.d[i]); // empty body intended
if (i < 0) {
return 

Update to the Double::toString(double) Javadoc

[raffaello . giulietti]

Hi,

as may be known, the current Javadoc for Double::toString(double) is not
specific enough. As a consequence, different implementations can return
different results.

To see this, here are some quotes from the Javadoc. It asks: "How many
digits must be printed for the fractional part of m or a?" It answers:
"There must be at least one digit to represent the fractional part, and
beyond that as many, but only as many, more digits as are needed to
uniquely distinguish the argument value from adjacent values of type
double."

One interpretation is: output "as many" digit of m (or a) itself. But
this can lead to unnecessarily long results. For example, the true value
of the double v closest to 0.3 is
v = 0.299988897769753748434595763683319091796875
so according to this restricted interpretation the method should return
"0.29998"
because shorter prefixes do not round to v.

Another interpretation exploits the astute word "represent" in the
answer, so it might sound: output "as many" digits of a nearby, vaguely
determined "short" number that "represents" m (or a) and happens to
round to v. In this case the obvious choice is
"0.3"
which is the result returned by the method.

One problem with this interpretation is that sometimes there is more
than one choice. For example, both 4.8E-324 and 4.9E-324 round to
Double.MIN_VALUE. The method chooses the latter one, presumably because
it is closer to the double, but this is not specified. It is also not
specified what happens if two equally shortest numbers that round to the
same double are also equally close to it.

Of course, the same holds for Float::toString(float).



The code that was uploaded to this mailing list [1] as a contribution to
fix [2], contains a purportedly better Javadoc that imposes a uniquely
specified result (see method math.DoubleToDecimal::toString(double) ).
It has been carefully written to ensure that the spirit of the current
Javadoc is preserved as much as possible and to make sure that the
results returned by the contributed code and by the current OpenJDK
implementation are consistent (modulo the anomalies discussed in [2]).



The question is if this needs to be submitted to the Compatibility &
Specification Review Group?
If so, what is the correct process to follow for an individual
contributor like me?

I ask because I would like my contribution to be accepted for the next
OpenJDK 11 LTS release: I have no idea on how long it would take for a
spec change to be approved, if the need arises at all in a case like
this one.



Greetings
Raffaello



[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-April/052696.html
[2] https://bugs.openjdk.java.net/browse/JDK-4511638

Re: Update to the Double::toString(double) Javadoc

[raffaello . giulietti]

Hi Joe,

so I beg Brian to get in touch with me for the CSR review.

I'm familiar with the paper of Steele and White.
For background on the design of my contribution see [1].


Greetings
Raffaello


[1] https://9b7022c5f9.files.wordpress.com/2018/04/todec.pdf


On 2018-04-27 20:58, joe darcy wrote:
> Hello Raffaello,
> 
> 
> On 4/27/2018 7:39 AM, raffaello.giulie...@gmail.com wrote:
>> Hi,
>>
>> as may be known, the current Javadoc for Double::toString(double) is not
>> specific enough. As a consequence, different implementations can return
>> different results.
>>
>> To see this, here are some quotes from the Javadoc. It asks: "How many
>> digits must be printed for the fractional part of m or a?" It answers:
>> "There must be at least one digit to represent the fractional part, and
>> beyond that as many, but only as many, more digits as are needed to
>> uniquely distinguish the argument value from adjacent values of type
>> double."
>>
>> One interpretation is: output "as many" digit of m (or a) itself. But
>> this can lead to unnecessarily long results. For example, the true value
>> of the double v closest to 0.3 is
>> v = 0.299988897769753748434595763683319091796875
>> so according to this restricted interpretation the method should return
>> "0.29998"
>> because shorter prefixes do not round to v.
>>
>> Another interpretation exploits the astute word "represent" in the
>> answer, so it might sound: output "as many" digits of a nearby, vaguely
>> determined "short" number that "represents" m (or a) and happens to
>> round to v. In this case the obvious choice is
>> "0.3"
>> which is the result returned by the method.
>>
>> One problem with this interpretation is that sometimes there is more
>> than one choice. For example, both 4.8E-324 and 4.9E-324 round to
>> Double.MIN_VALUE. The method chooses the latter one, presumably because
>> it is closer to the double, but this is not specified. It is also not
>> specified what happens if two equally shortest numbers that round to the
>> same double are also equally close to it.
>>
>> Of course, the same holds for Float::toString(float).
> 
> For background on the design of this printing methodology see  "How to
> Print Floating-Point Numbers Accurately," Guy L. Steele and Jon L. White
> in the proceedings of the 1990 PLDI.
> 
>> The code that was uploaded to this mailing list [1] as a contribution to
>> fix [2], contains a purportedly better Javadoc that imposes a uniquely
>> specified result (see method math.DoubleToDecimal::toString(double) ).
>> It has been carefully written to ensure that the spirit of the current
>> Javadoc is preserved as much as possible and to make sure that the
>> results returned by the contributed code and by the current OpenJDK
>> implementation are consistent (modulo the anomalies discussed in [2]).
>>
>>
>>
>> The question is if this needs to be submitted to the Compatibility &
>> Specification Review Group?
>> If so, what is the correct process to follow for an individual
>> contributor like me?
> 
> It does sound like the change merits CSR review.  On a procedural front,
> to create a CSR, one needs an account in the bug system, so the sponsor
> of your issue with an account, Brian Burkhalter I believe, would need to
> assist here. Conceptually, the CSR process is described on its wiki page
> https://wiki.openjdk.java.net/display/csr/Main. The review is concerned
> with evaluating the compatibility impact of the spec (or behavioral
> change), so information about those matters is provided and considered.
> 
>>
>> I ask because I would like my contribution to be accepted for the next
>> OpenJDK 11 LTS release: I have no idea on how long it would take for a
>> spec change to be approved, if the need arises at all in a case like
>> this one.
>>
> 
> The normal review SLA for CSR review is a week. Note this is time to
> review rather than time to approve since the CSR can have comments that
> need to be addressed, etc.
> 
> HTH and thanks for the code submission,
> 
> -Joe

speedup factors of the proposed re-implementation of Double.toString(double)

[raffaello . giulietti]

Hi,

in order to elicit some concrete interest in the proposed
re-implementation of Double.toString(double) [1] to overcome the known
issues [2], I prepared a jar in [3] that runs some benchmarks comparing
the execution times.

It can be executed directly as:
java --module-path todecbench.jar -m todec

The source code has already been delivered in [4].

The proposed implementation corrects all the known anomalies and the
benchmarks show that it is faster.

While I have no opportunities to run the benchmarks on idle server class
hardware, the speedup factors obtained on my not so recent laptop
(i7-3630QM @ 2.4 GHz, 12 GiB RAM, Windows 10 Pro 64 bit, Oracle java
9.0.4 64 bit) are:

* 11x for non NaN values uniformly distributed over the whole range of
doubles
* 2x for doubles of the form (int)/1e3 ("milli") and (int)/1e6 ("micro")
* on par with doubles of the form (int)

Not shown in the benchmark, but collected over extensive testing, are
speedup factors in-between.

In about 99.4% of the cases no objects are ever allocated (except for
the final result), which helps reducing gc costs. For details about the
algorithmic part see the pdf in [3].

Hope this helps in getting some of you to try the code and give feedback.



Greetings
Raffaello


[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/052355.html
[2] https://bugs.openjdk.java.net/browse/JDK-4511638
[3]
https://drive.google.com/drive/folders/1MErGSOaqDFw2q9rz48WlpBDKyZkLuUqj?usp=sharing
[4]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-April/052696.html

Comment on 8260692: Matematically wrong implementations of RoundingMode.HALF_UP and .HALF_DOWN

[Raffaello Giulietti]

Hi,

in my opinion, and contrary to the report [1], the described issue is 
not a bug.


While the wording used in the spec of java.math.RoundingMode could be 
improved, it is nonetheless clear enough to avoid any confusion about 
what HALF_DOWN and HALF_UP mean in this context. The actual values 
reported are all correct while the expected ones are not.


I think the issue can be closed as not relevant.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8260692

Comment on 8259896: Base64 MIME decoder should allow unrecognised characters within padding

[Raffaello Giulietti]

Hi,

in my opinion, the reporter of [1] is right in requiring that extraneous 
characters be discarded, even inside the padding.


Indeed, the first full paragraph on [2] reads:

"Any characters outside of the base64 alphabet are to be ignored in
base64-encoded data."

where "the base64 alphabet" also includes the padding character '=' and 
"base64-encoded data" extends to padding as well, because padding is an 
essential part of encoding.


The legitimate doubt expressed in comment [3] should thus be solved in 
favor of a bug fix.



My 2 cents
Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8259896
[2] https://tools.ietf.org/html/rfc2045#page-26
[3] 
https://bugs.openjdk.java.net/browse/JDK-8259896?focusedCommentId=14395485&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14395485

RFR: 8240632: Note differences between IEEE 754-2019 math lib special cases and java.lang.Math

[Raffaello Giulietti]

Hello Joe,

in the javadoc of acos() I think you should replace the mathematically 
incorrect


 * If the argument is {@linkplain Double#isInfinite infinite},

with something like

 * If the argument is zero,



Also, in some places pi/2 is described as

pi/2, rounded,

which is more open to interpretation, depending on what is meant by 
"rounded" (which mode? Granted, default is "to closest"), whereas in 
other places it is described as


the value closest to pi/2

which is preferable, as it is unambiguous.


Greetings
Raffaello




On Thu, 4 Feb 2021 02:08:12 GMT, Joe Darcy  wrote:


Recent revisions of the IEEE 754 floating-point standard have added guidance on 
how typical math library methods (sin, cos, tan, etc.) should behave in terms 
of their general quality of implementation as well as on special values.

Other than the pow methods, for the recommended operations listed by IEEE 754 
that the Java math library already includes, the special cases that are 
specified by Java are the same as those specified by IEEE 754, except for the 
pow method. IEEE 754 calls out some special cases not explicitly listed in the 
Java specs. This changeset adds those special cases to the spec and adds tests 
of the the special cases if not already present.

If method "Foo" already had a regression test, new cases were added it it. 
Otherwise, a new test was added to cover the special cases of several methods.

There is no intention at the moment to change the behavior of pow to align with 
IEEE 754.


PS After the wording for the Math class is finalized, I'll add analogous 
wording to the StrictMath class and run a CSR.

-

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

Fast and cheap (Double|Float)::toString Java algorithm

[Raffaello Giulietti]

Hello,

as a reminder, a Java implementation of Schubfach [1] intended to 
replace the current slow and expensive OpenJDK (Double|Float)::toString 
algorithm has been discussed, presented and contributed to this mailing 
list in several posts. The last implementation is available in pre-Skara 
webrev form, as referenced in [2]. On my laptop hardware, the speedup 
factor is 17.7x wrt OpenJDK.




Recently, Alexander Bolz translated Schubfach to C++ for the purpose of 
comparing the performance of several bin2dec floating-point numbers 
algorithms [3].


In the meantime, Junekey Jeon implemented and perfected his own 
Dragonbox in C++, a new algorithm based on Schubfach [4]. From [5]: "In 
addition to the core idea of Schubfach, Dragonbox utilizes some 
Grisu-like ideas to minimize the number of expensive 128-bit × 64-bit 
multiplications, at the cost of having more branches and 
divisions-by-constants."


In the C++ ecosystem, Schubfach has been the fastest known algorithm 
before being gently pushed aside by Dragonbox, as can be seen in the 
graphs in [3].




While developing Schubfach back in 2018, I experimented myself with 
blending core Schubfach with my own earlier algorithm similar to Grisu. 
However, probably due to uncontrollable JIT compilation behavior, I 
could not observe any benefit. On the contrary, I measured performance 
drops probably because of the added complexity, which is public enemy 
nr. 1 for JIT compilers. Therefore, I opted for the simpler current 
design that seemed more suitable for (the then 2018 version of) C2.




I hope this can somehow revive this community's interest and confidence 
toward Schubfach to definitely supplant the current expensive 
(Double|Float)::toString algorithm.



Greetings
Raffaello



[1] https://drive.google.com/open?id=1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065297.html

[3] https://github.com/abolz/Drachennest
[4] https://github.com/jk-jeon/dragonbox
[5] 
https://github.com/jk-jeon/dragonbox/blob/master/other_files/Dragonbox.pdf

Contributing to 8259896: Base64 MIME decoder should allow unrecognised characters within padding

[Raffaello Giulietti]

Hello,

I can contribute in solving the issue [1] mentioned in [2].

But before investing time, I just want to make sure that there is some 
consensus that it needs to be solved in the first place. Maybe there are 
diverging opinions that consider the current behavior as correct.



Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8259896
[2] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-February/074407.html




Hello Raffaello,

I have added this comment from you, to that JDK-8259896 issue.

-Jaikiran

On 02/02/21 12:43 am, Raffaello Giulietti wrote:

Hi,

in my opinion, the reporter of [1] is right in requiring that 
extraneous characters be discarded, even inside the padding.


Indeed, the first full paragraph on [2] reads:

"Any characters outside of the base64 alphabet are to be ignored in
base64-encoded data."

where "the base64 alphabet" also includes the padding character '=' 
and "base64-encoded data" extends to padding as well, because padding 
is an essential part of encoding.


The legitimate doubt expressed in comment [3] should thus be solved in 
favor of a bug fix.



My 2 cents
Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8259896
[2] https://tools.ietf.org/html/rfc2045#page-26
[3] 
https://bugs.openjdk.java.net/browse/JDK-8259896?focusedCommentId=14395485&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14395485

Comment on CSR 8251991: Hex formatting and parsing utility

[Raffaello Giulietti]

Hello,

it appears that all of the following API methods in [1] can be declared 
*static*, which makes them more useful in contexts where there's no 
instance of HexFormat available or none is desired.


As 17 has not yet entered any formal phase, the change should be harmless.

  public boolean isHexDigit(int);
  public int fromHexDigit(int);
  public int fromHexDigits(java.lang.CharSequence);
  public int fromHexDigits(java.lang.CharSequence, int, int);
  public long fromHexDigitsToLong(java.lang.CharSequence);
  public long fromHexDigitsToLong(java.lang.CharSequence, int, int);

Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8251991

Re: Comment on CSR 8251991: Hex formatting and parsing utility

[Raffaello Giulietti]

Hi,

the javadoc of most of the methods listed in my previous post below reads:
"The delimiter, prefix, suffix, and uppercase parameters are not used."

As these eventually constitute the whole state of an instance of this 
final class and as the state is not involved, it is possible to declare 
the methods as static.



If, for some reason, declaring the methods as static is not a choice, 
the next best thing is to clarify that

* HexFormat.of() always returns the same instance and thus
* absent any other instance, the recommended idiom for invoking any of 
the methods below is, for example,

HexFormat.of().isHexDigit('F')
(because there are almost no additional costs wrt static methods.)

But I don't see a reason why they should be non-static. Just oversight?


Greetings
Raffaello


On 2021-03-08 11:52, Raffaello Giulietti wrote:

Hello,

it appears that all of the following API methods in [1] can be declared 
*static*, which makes them more useful in contexts where there's no 
instance of HexFormat available or none is desired.


As 17 has not yet entered any formal phase, the change should be harmless.

   public boolean isHexDigit(int);
   public int fromHexDigit(int);
   public int fromHexDigits(java.lang.CharSequence);
   public int fromHexDigits(java.lang.CharSequence, int, int);
   public long fromHexDigitsToLong(java.lang.CharSequence);
   public long fromHexDigitsToLong(java.lang.CharSequence, int, int);

Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8251991

Re: Comment on CSR 8251991: Hex formatting and parsing utility

[Raffaello Giulietti]

Hello,

I understand your points expressed in 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-October/070317.html


On the other hand, it kind of hurts when the javadoc spec implies that 
the fromXXX() methods do not logically depend on the state and yet one 
has to invoke them on some instance anyway.



As for HexFormat.of() to always return the same instance, you are of 
course right in observing that this is out of place for value-based 
classes. The intent was to describe a static-like idiom that would deter 
programmers to rewrite their own static version of these methods.



Greetings
Raffaello



Hi,

The question about static vs instance methods was raised during the review.
At the time, my rationale for the current API is summarized in this email.
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-October/070317.html

It comes down to choosing one kind of consistency vs another.

I was/am looking for additional comments to weigh one or the other 
alternative.
I did receive one offline comment in support of static methods on the 
rationale of

convenience.

Regards, Roger


On 3/15/21 12:06 PM, Raffaello Giulietti wrote:

Hi,

the javadoc of most of the methods listed in my previous post below 
reads:
"The delimiter, prefix, suffix, and uppercase parameters are not 
used."


As these eventually constitute the whole state of an instance of this 
final class and as the state is not involved, it is possible to 
declare the methods as static.



If, for some reason, declaring the methods as static is not a choice, 
the next best thing is to clarify that

* HexFormat.of() always returns the same instance and thus
* absent any other instance, the recommended idiom for invoking any of 
the methods below is, for example,

HexFormat.of().isHexDigit('F')
(because there are almost no additional costs wrt static methods.)

Note that HexFormat is specified as a value based class and
any statement related to identity is out of place.

Incidentally, its generally discouraged and can cause a warning when a 
static method is invoked using a reference.




But I don't see a reason why they should be non-static. Just oversight?


Greetings
Raffaello


On 2021-03-08 11:52, Raffaello Giulietti wrote:

Hello,

it appears that all of the following API methods in [1] can be 
declared *static*, which makes them more useful in contexts where 
there's no instance of HexFormat available or none is desired.


As 17 has not yet entered any formal phase, the change should be 
harmless.


   public boolean isHexDigit(int);
   public int fromHexDigit(int);
   public int fromHexDigits(java.lang.CharSequence);
   public int fromHexDigits(java.lang.CharSequence, int, int);
   public long fromHexDigitsToLong(java.lang.CharSequence);
   public long fromHexDigitsToLong(java.lang.CharSequence, int, int);

Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8251991

Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Hello,

I'm experimenting with an implementation of Decimal64 and Decimal128, 
two standard IEEE 754-2019 formats supporting, well, decimal floating 
point numbers of up to 16 resp 34 decimal digits.


While BigDecimal can simulate most finite number computations on these 
formats by passing MathContext.DECIMAL64 and MathContext.DECIMAL128, 
resp., to the arithmetic operations, this approach has some shortcomings 
wrt to the standard:
* Like the binary formats (float and double in Java), the standard 
mandates signed zeroes, signed infinities and NaNs for the decimal 
formats as well, while BigDecimal has neither.

* BigDecimal is not a final class, so is not (and can never be) value-based.
* BigDecimal needs support from BigInteger (another non final, non 
value-based class).


On the other hand, classes Decimal64 and Decimal128:
* Are value-based and can be declared to be primitive in a future 
version of OpenJDK, once Valhalla is completed.
* Require 12 and 20 bytes, resp, of storage in the Valhalla model of 
primitive objects and have no references to other identity or primitive 
objects.

* Support the signed zeroes, the infinities and the NaNs.
* Currently support the 4 fundamental operations (+ - * /) according to 
the standard and will support remainder, square root, comparisons, etc. 
and the conversions listed in the standard.
* Could be extended to support the (rather cumbersome) exceptions 
handling of the standard.
* There's no plan to support transcendental functions, as the aim is to 
have numerical classes for business applications, not scientific or 
engineering purposes.
* Are faster than the (incomplete) simulation with BigDecimal, 1x-5x in 
the current incarnations.



I would be glad to contribute the code to the OpenJDK provided there is 
a genuine interest from the community and a reviewer willing to sponsor 
my effort. (I guess the best candidate for this role would be Joe Darcy 
from what I can read in this mailing list.)



Greetings
Raffaello

Re: Comment on CSR 8251991: Hex formatting and parsing utility

[Raffaello Giulietti]

Thanks, I noticed.

There are a couple of other methods that can be simplified, require a 
couple of minutes to review and which don't have any impact on the API.


If you agree, I'll prepare a PR with all the changes and the bug 
narrative for you to add to bugs.openjdk.org (I only have read access).



Greetings
Raffaello



On 2021-03-26 16:08, Roger Riggs wrote:

fyi,  a PR to make the methods static.

https://git.openjdk.java.net/jdk/pull/3205

On 3/15/21 4:26 PM, Raffaello Giulietti wrote:

Hello,

I understand your points expressed in 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-October/070317.html 



On the other hand, it kind of hurts when the javadoc spec implies that 
the fromXXX() methods do not logically depend on the state and yet one 
has to invoke them on some instance anyway.



As for HexFormat.of() to always return the same instance, you are of 
course right in observing that this is out of place for value-based 
classes. The intent was to describe a static-like idiom that would 
deter programmers to rewrite their own static version of these methods.



Greetings
Raffaello



Hi,

The question about static vs instance methods was raised during the 
review.
At the time, my rationale for the current API is summarized in this 
email.
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-October/070317.html 



It comes down to choosing one kind of consistency vs another.

I was/am looking for additional comments to weigh one or the other 
alternative.
I did receive one offline comment in support of static methods on the 
rationale of

convenience.

Regards, Roger


On 3/15/21 12:06 PM, Raffaello Giulietti wrote:

Hi,

the javadoc of most of the methods listed in my previous post below 
reads:
    "The delimiter, prefix, suffix, and uppercase parameters are not 
used."


As these eventually constitute the whole state of an instance of 
this final class and as the state is not involved, it is possible to 
declare the methods as static.



If, for some reason, declaring the methods as static is not a 
choice, the next best thing is to clarify that

* HexFormat.of() always returns the same instance and thus
* absent any other instance, the recommended idiom for invoking any 
of the methods below is, for example,

    HexFormat.of().isHexDigit('F')
(because there are almost no additional costs wrt static methods.)

Note that HexFormat is specified as a value based class and
any statement related to identity is out of place.

Incidentally, its generally discouraged and can cause a warning when 
a static method is invoked using a reference.




But I don't see a reason why they should be non-static. Just oversight?


Greetings
Raffaello


On 2021-03-08 11:52, Raffaello Giulietti wrote:

Hello,

it appears that all of the following API methods in [1] can be 
declared *static*, which makes them more useful in contexts where 
there's no instance of HexFormat available or none is desired.


As 17 has not yet entered any formal phase, the change should be 
harmless.


   public boolean isHexDigit(int);
   public int fromHexDigit(int);
   public int fromHexDigits(java.lang.CharSequence);
   public int fromHexDigits(java.lang.CharSequence, int, int);
   public long fromHexDigitsToLong(java.lang.CharSequence);
   public long fromHexDigitsToLong(java.lang.CharSequence, int, int);

Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8251991

Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Hi,

the intent of this proposal is to have numerical classes for *business 
and financial applications*, not to implement the IEEE 754-2019 spec for 
the sake of full coverage.


The goal is to have numerical classes that are lighter-weight and more 
efficient in terms of storage space and computation time wrt BigDecimal 
*and* to adhere to a widespread standard whose model is familiar to many 
programmers.


Perhaps some extended precision decimal format could be handy. On the 
other hand, I don't see why binary floating point numbers would be a 
useful option in this area.



Greetings
Raffaello




This would be interesting to have.

It would be better if the full set of numbers b16-256 and d32-128 are
implemented so there is full coverage of all IEEE 754 numbers.


On Tue, 30 Mar 2021 at 01:45, Raffaello Giulietti <
raffaello.giulietti at gmail.com> wrote:


Hello,

I'm experimenting with an implementation of Decimal64 and Decimal128,
two standard IEEE 754-2019 formats supporting, well, decimal floating
point numbers of up to 16 resp 34 decimal digits.

While BigDecimal can simulate most finite number computations on these
formats by passing MathContext.DECIMAL64 and MathContext.DECIMAL128,
resp., to the arithmetic operations, this approach has some shortcomings
wrt to the standard:
* Like the binary formats (float and double in Java), the standard
mandates signed zeroes, signed infinities and NaNs for the decimal
formats as well, while BigDecimal has neither.
* BigDecimal is not a final class, so is not (and can never be)
value-based.
* BigDecimal needs support from BigInteger (another non final, non
value-based class).

On the other hand, classes Decimal64 and Decimal128:
* Are value-based and can be declared to be primitive in a future
version of OpenJDK, once Valhalla is completed.
* Require 12 and 20 bytes, resp, of storage in the Valhalla model of
primitive objects and have no references to other identity or primitive
objects.
* Support the signed zeroes, the infinities and the NaNs.
* Currently support the 4 fundamental operations (+ - * /) according to
the standard and will support remainder, square root, comparisons, etc.
and the conversions listed in the standard.
* Could be extended to support the (rather cumbersome) exceptions
handling of the standard.
* There's no plan to support transcendental functions, as the aim is to
have numerical classes for business applications, not scientific or
engineering purposes.
* Are faster than the (incomplete) simulation with BigDecimal, 1x-5x in
the current incarnations.


I would be glad to contribute the code to the OpenJDK provided there is
a genuine interest from the community and a reviewer willing to sponsor
my effort. (I guess the best candidate for this role would be Joe Darcy
from what I can read in this mailing list.)


Greetings
Raffaello

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Hi Joe,

On 2021-03-30 21:56, Joe Darcy wrote:

Hi Raffaello,

On 3/29/2021 1:14 PM, Raffaello Giulietti wrote:

Hello,



Assuming you have DecimalN <-> BigDecimal conversions, the BigDecimal 
type should be usable for testing at least. For in-range values not near 
the exponent range, the scale (exponent) and significand for finite and 
nonzero values should be the same for the basic arithmetic operations 
and square root in DecimalN and BigDecimal.


(Around the limits of the exponent range, there are subnormal and 
"supernormal" results where the rounding of the significand interacts 
with the exponent computation. This would make it a bit tricky to 
offload BigDecimal computations in range of, say, a Decimal128 to a 
hardware implementation where one was available.)




Yes, some of my current tests exploit BigDecimal for crosschecks in the 
normal range.



Fixed-size decimal computations have an interesting design space to 
explore in terms of trading off memory compactness and computational 
cost. For example, the IEEE 754 spec details two different encodings of 
three decimal digits in 10 bits. However, it is not strictly necessary 
for each operation to produce results whose internal storage maps to an 
"interchange format," in the terminology of 754. It would be possible to 
keep the significand encoded as normal integers and only produce an 
interchange representation on something akin to a serialization event.




The internal representation is none of the interchange formats. 
Similarly to the interchange format's 1'000-based declets, it holds 
1'000'000'000-based (30 bits) "declets"/int, in addition to the unbiased 
exponent, the sign, the precision and the kind, as you mention below.



I understand hardware implementations of FPUs will use these sort of 
techniques and also have architectural-invisible bits indicating what 
kind of value a floating-point number is. For example, the beginning of 
many math library functions starts with "Handle NaN cases," ... "Handle 
infinity case" ... "Handle zero case" ... Sometimes the handling falls 
out naturally and doesn't require explicit testing, but in the cases 
that does not occur, it can be cheaper to have the "isFoo" bit already 
computed.



I would be glad to contribute the code to the OpenJDK provided there 
is a genuine interest from the community and a reviewer willing to 
sponsor my effort. (I guess the best candidate for this role would be 
Joe Darcy from what I can read in this mailing list.)


At least for the time being, I don't think the OpenJDK (at least the 
main project) would be the best home for such code. As you're noted, 
post Valhalla having such a type in the JDK becomes more interesting 
from a performance perspective.




They are already more compact and faster than BigDecimal even on the 
current, pre-Valhalla release, so they would make sense even today, were 
they ready for review.


(I'm spending my free time on this, so I just wanted to make sure I'm 
not wasting energy for something that will sit on my computer only.)



Greetings
Raffaello

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

There's no strict need for Valhalla, as they are more compact and faster 
than BigDecimal even today.



On 2021-03-30 22:03, Brian Goetz wrote:
Overall, I'd be happy to see Decimal types that are aimed at "reasonable 
precision" in addition to the infinite precision that BD offers.  (After 
Valhalla, of course.)




On 3/29/2021 4:14 PM, Raffaello Giulietti wrote:

Hello,

I'm experimenting with an implementation of Decimal64 and Decimal128, 
two standard IEEE 754-2019 formats supporting, well, decimal floating 
point numbers of up to 16 resp 34 decimal digits.

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Hi Paul,


On 2021-03-30 22:54, Paul Sandoz wrote:




On Mar 30, 2021, at 1:03 PM, Brian Goetz  wrote:

Overall, I'd be happy to see Decimal types that are aimed at "reasonable 
precision" in addition to the infinite precision that BD offers.  (After Valhalla, 
of course.)



Yes, me too.

Raffaello, as an experiment you could develop such classes as primitive classes 
compiled against recent builds of Valhalla.



I guess the most recent builds of Valhalla are those resulting from 
building from the source code in the git repo. Or are you recommending 
the official ea releases instead?


I ask because I plan to use the latest release from the git repo and 
keeping it current, not the ea release.




I would caution against unduly biasing towards "business applications”, as I 
think with primitive classes and other possible features Java can become a better 
platform for scientific or engineering purposes.

As an example we are making progress with the Vector API and in the 
panama-vector repo we have integrated with code of Intel’s Short Vector Math 
Library, which may open the possibility for it to be used by the 
auto-vectorizer too.

Paul.



As far as I can tell, scientific computation will make use of binary 
floating point numbers for a long time. Decimal floating point numbers 
are still limited to biz and fin applications.


Does Intel's lib operates on decimal formats as well?


Greetings
Raffaello

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Ciao Maurizio,

thanks for your interest.


On 2021-03-30 23:01, Maurizio Cimadamore wrote:

Add me to the list of interested folks.



Will do.


Such types are useful when interacting with System ABIs, and the Foreign 
Linker API needs "well-known" carrier which model these types in Java. 
There are also other interesting types to be explored in that domain, 
such as Long128, LongDouble (extended precision float) and HalfFloats.

 > Maurizio

On 30/03/2021 21:54, Paul Sandoz wrote:



On Mar 30, 2021, at 1:03 PM, Brian Goetz  wrote:

Overall, I'd be happy to see Decimal types that are aimed at 
"reasonable precision" in addition to the infinite precision that BD 
offers.  (After Valhalla, of course.)



Yes, me too.

Raffaello, as an experiment you could develop such classes as 
primitive classes compiled against recent builds of Valhalla.


I would caution against unduly biasing towards "business 
applications”, as I think with primitive classes and other possible 
features Java can become a better platform for scientific or 
engineering purposes.


As an example we are making progress with the Vector API and in the 
panama-vector repo we have integrated with code of Intel’s Short 
Vector Math Library, which may open the possibility for it to be used 
by the auto-vectorizer too.


Paul.

Further review of java.util.HexFormat

[Raffaello Giulietti]

Hello Roger,


these are the changes I'm proposing after reviewing the code of 
j.u.HexFormat. The modified code available here

https://github.com/rgiulietti/jdk/commit/6759a25eb952ab19a045a83349d41b82cc1b07cb

In addition to other smaller, hopefully self-explanatory enhancements, 
here are the rationales for the changes.



Static field DIGITS should preferably be formatted with 16 values/line 
to ease a visual positional crosscheck with published ASCII/IsoLatin1 
tables.



Field digits is initialized with either UPPERCASE_DIGITS, 
LOWERCASE_DIGITS or digits from another instance, so it always ends up 
being either UPPERCASE_DIGITS or LOWERCASE_DIGITS.

Consequently:
* There's no need for requireNonNull() check in the (sole) private 
constructor.
* It's preferable to move the last comparison in method equals() as the 
first factor in the return statement, so it can return faster in case of 
a lower/upper mismatch. (Arrays.equals() first checks for ==, so it 
always returns fast as used in this class. It could even be replaced by 
a simple == )



Method fromHexDigits(CharSequence, int) either returns a value in the 
range [0x00..0xff] or throws.
Therefore, there's no need for the checks leading to the throwing of 
IllegalArgumentException in methods

* parseHex(CharSequence, int, int)
* parseNoDelimiter(CharSequence)
which can be simplified as a consequence.


The test for IllegalArgumentException in method parseHex(CharSequence, 
int, int), namely

string.length() < valueChars || (string.length() - valueChars) % stride != 0
can be simplified as
(string.length() - valueChars) % stride != 0

Indeed, at this point in the control flow we have
string.length() > 0  and  stride >= valueChars
Assuming string.length() < valueChars as in the left operand of || we 
then have

-stride <= -valueChars < string.length() - valueChars < 0
so
string.length() - valueChars) % stride != 0
which is the right operand of ||.
In other words, the left operand always implies the right one, adding 
nothing to it.



There's no need to check again for non-nullness in private method 
fromHexDigits(CharSequence, int). It is invoked from two places where 
the check is already performed.



Both fromHexDigits(CharSequence) and fromHexDigitsToLong(CharSequence) 
can simply invoke their 3 args counterparts.



If you prefer, I can prepare a PR once there's an issue in the bug 
system to associate the PR with.



Greetings
Raffaello

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Hi,

I think there's a misunderstanding about the nature of IEEE 754 
Decimal (e.g., Decimal64), the subject of this thread, and the nature 
of SQL DECIMAL(p, s).


SQL DECIMAL(p, s) represent *fixed* point decimal numbers, with an 
overall maximum precision p and a scale s, the number of digits to the 
right of the decimal point (both parameters can be selected freely 
inside some ranges). For example, DECIMAL(2, 1) can represent only the 
values

-9.9, -9.8, ..., 9.8, 9.9
and that's it.
Thus, the sum 6.6 + 7.7 overflows, as well as the product 2.9 * 4.

IEEE decimals are *floating* point decimal numbers. A hypothetical 
decimal of precision 2 can represent values of the form c*10^q, where 
integer c meets |c| < 100 (that is, max two digits) and integer q is 
limited in some range. It covers the values above and much more, for 
example, 0.012 (=12*10^(-3)) and -3.4E2 (=-34*10^1).
The sum 6.6 + 7.7 produces 14 because the mathematical result 14.3 is 
rounded to the closest number of precision 2 (assuming 
RoundingMode.HALF_EVEN). By the same token, the product 2.9 * 4 produces 
12, which is 11.6 rounded to 2 digits.

But really, the position of the decimal point is floating.

IEEE decimals and SQL decimals are fundamentally different and ave 
different arithmetic, so I wouldn't recommend using the proposed classes 
for JDBC.


On the positive side, SQL decimals, are easier to implement if the 
maximum allowed p in DECIMAL(p, s) is reasonable, say 38. But that's 
another topic.



Greetings
Raffaello

Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Ciao Maurizio,

admittedly, yours is a fairly convincing argument to wait for the 
completion of Valhalla, or at least JEP 401.


Personally, I wouldn't mind having to denote the primitive class as 
Decimal128.val in some future (2022? 2023? who knows?) if I could use 
Decimal128 tomorrow, but I understand your concerns in defending the 
interests of the community at large (which includes myself).



Greetings
Raffaello



On 31/03/2021 15:23, Douglas Surber wrote:

Rather than waiting on Valhala I would prefer that this project be fast tracked 
and added to OpenJDK ASAP.


There is a catch here.

While in principle, we can add these as value-based classes, and migrate 
to Valhalla later, there is a biggie difference between doing it 
before/after.


When it comes to "migrated" primitive classes, there is a choice in how 
to interpret the "old" utterances of the class name. Let's say that 
class Foo is migrated to be a primitive class; does that mean that all 
uses of Foo in existing program will automatically get flattening? Or 
will references to Foo be interpreted in a conservative fashion, so  as 
to allow the same operations as before? One important difference in 
semantics is assignment to `null` which is prohibited under flattened 
semantics, but allowed under "indirect" (or by reference, if you will) 
semantics.


In other words, under the current plan, if Decimal128 is added now and 
migrated later, utterances of Decimal128 will behave like they used to 
pre-Valhalla, and, to take advantage of flattening you would need to 
opt-in with some keyword (e.g. Decimal128.val).


To me this is kind of a strong argument against going with these classes 
now (as much as I understand how useful they'd be even w/o Valhalla) - 
and preserving the "good" name (Decimal128) for the flattened case seems 
worth, IMHO, waiting few more cycles.


Maurizio

Re: [External] : Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

Hi Douglas,

yes, different vendors have different limits on the precision, the most 
extreme probably being PostgreSQL.


But apart from that, the arithmetic is different.

A better option is to implement some optimized fixed precision classes 
like SQLDecimal38 and SQLDecimal65 + a more general variable precision 
SQLDecimal. But, as I mentioned, this is something different than 
Decimal.



Greetings
Raffaello



On 2021-03-31 22:53, Douglas Surber wrote:

Understood. The problem is that right now the only appropriate type for 
non-integer SQL numbers is BigDecimal. It's too big and too slow and lots of 
users avoid it.

Decimal128 supports 34 significant digits. The max precision of SQL numeric 
types varies from vendor to vendor. In SQL Server it is 38. In MySQL it is 65. 
So there are a huge range of values representable in SQL that are not 
representable in Decimal128. BUT, for the vast majority of applications that 
might be tempted to use Decimal128, those non-representable numbers don't 
occur. Currency amounts exceeding 34 decimal digits of precision are an almost 
non-existent minority.

Very few apps will pay the price of using BigDecimal even though it would 
support huge numbers exactly. Instead they find workarounds that are more 
efficient. Decimal128 would be a substantial improvement for those apps.

Douglas


On Mar 31, 2021, at 1:13 PM, Raffaello Giulietti 
 wrote:

Hi,

I think there's a misunderstanding about the nature of IEEE 754 Decimal 
(e.g., Decimal64), the subject of this thread, and the nature of SQL DECIMAL(p, s).

SQL DECIMAL(p, s) represent *fixed* point decimal numbers, with an overall 
maximum precision p and a scale s, the number of digits to the right of the 
decimal point (both parameters can be selected freely inside some ranges). For 
example, DECIMAL(2, 1) can represent only the values
-9.9, -9.8, ..., 9.8, 9.9
and that's it.
Thus, the sum 6.6 + 7.7 overflows, as well as the product 2.9 * 4.

IEEE decimals are *floating* point decimal numbers. A hypothetical decimal of 
precision 2 can represent values of the form c*10^q, where integer c meets |c| 
< 100 (that is, max two digits) and integer q is limited in some range. It 
covers the values above and much more, for example, 0.012 (=12*10^(-3)) and -3.4E2 
(=-34*10^1).
The sum 6.6 + 7.7 produces 14 because the mathematical result 14.3 is rounded 
to the closest number of precision 2 (assuming RoundingMode.HALF_EVEN). By the 
same token, the product 2.9 * 4 produces 12, which is 11.6 rounded to 2 digits.
But really, the position of the decimal point is floating.

IEEE decimals and SQL decimals are fundamentally different and ave different 
arithmetic, so I wouldn't recommend using the proposed classes for JDBC.

On the positive side, SQL decimals, are easier to implement if the maximum 
allowed p in DECIMAL(p, s) is reasonable, say 38. But that's another topic.


Greetings
Raffaello

RFR: 8264514: HexFormat implementation tweaks

[Raffaello Giulietti]

Please review these small tweaks.

For background information see 
[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html)

Greetings
Raffaello

-

Commit messages:
 - 8264514: HexFormat implementation tweaks

Changes: https://git.openjdk.java.net/jdk/pull/3295/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3295&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264514
  Stats: 67 lines in 1 file changed: 10 ins; 26 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3295.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3295/head:pull/3295

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

Re: [External] : Re: Proposal for Decimal64 and Decimal128 value-based classes

[Raffaello Giulietti]

The issue is to simulate SQL DECIMAL arithmetic using IEEE Decimal

Consider the division 20 / 3. Using Decimal64:
20 / 3 -> 6.7
Using DECIMAL(38, 6), or many other reasonable combinations:
20 / 3 -> 6.67

We could expose a richer API in Decimal, similar to BigDecimal's. It 
is doable, but beyond IEEE and my current plan.


In addition, I find that the point issued by Maurizio about the best 
name for the Valhalla primitive class is important enough for the 
community at large to warrant a rumination pause.


Let me ponder about how to enrich the API to support SQL for a couple of 
days or so. In the meantime, the Valhalla guys may come up with a good 
solution to better assist migration of @ValueBased classes to truly 
primitive classes without disrupting existing code that depends on them, 
as described by Maurizio in his previous post.




On 2021-03-31 23:24, Douglas Surber wrote:

My understanding is that IEEE decimal floating point is intended for currency. 
A large fraction of numeric values stored in databases are currency. It's not 
obvious to me why an e-commerce web site would not want to use Decimal128 to 
represent prices, extensions, taxes, discounts, totals, etc.


On Mar 31, 2021, at 2:17 PM, Raffaello Giulietti 
 wrote:

Hi Douglas,

yes, different vendors have different limits on the precision, the most extreme 
probably being PostgreSQL.

But apart from that, the arithmetic is different.

A better option is to implement some optimized fixed precision classes like 
SQLDecimal38 and SQLDecimal65 + a more general variable precision SQLDecimal. But, as 
I mentioned, this is something different than Decimal.


Greetings
Raffaello



On 2021-03-31 22:53, Douglas Surber wrote:

Understood. The problem is that right now the only appropriate type for 
non-integer SQL numbers is BigDecimal. It's too big and too slow and lots of 
users avoid it.
Decimal128 supports 34 significant digits. The max precision of SQL numeric 
types varies from vendor to vendor. In SQL Server it is 38. In MySQL it is 65. 
So there are a huge range of values representable in SQL that are not 
representable in Decimal128. BUT, for the vast majority of applications that 
might be tempted to use Decimal128, those non-representable numbers don't 
occur. Currency amounts exceeding 34 decimal digits of precision are an almost 
non-existent minority.
Very few apps will pay the price of using BigDecimal even though it would 
support huge numbers exactly. Instead they find workarounds that are more 
efficient. Decimal128 would be a substantial improvement for those apps.
Douglas

On Mar 31, 2021, at 1:13 PM, Raffaello Giulietti 
 wrote:

Hi,

I think there's a misunderstanding about the nature of IEEE 754 Decimal 
(e.g., Decimal64), the subject of this thread, and the nature of SQL DECIMAL(p, s).

SQL DECIMAL(p, s) represent *fixed* point decimal numbers, with an overall 
maximum precision p and a scale s, the number of digits to the right of the 
decimal point (both parameters can be selected freely inside some ranges). For 
example, DECIMAL(2, 1) can represent only the values
-9.9, -9.8, ..., 9.8, 9.9
and that's it.
Thus, the sum 6.6 + 7.7 overflows, as well as the product 2.9 * 4.

IEEE decimals are *floating* point decimal numbers. A hypothetical decimal of 
precision 2 can represent values of the form c*10^q, where integer c meets |c| 
< 100 (that is, max two digits) and integer q is limited in some range. It 
covers the values above and much more, for example, 0.012 (=12*10^(-3)) and -3.4E2 
(=-34*10^1).
The sum 6.6 + 7.7 produces 14 because the mathematical result 14.3 is rounded 
to the closest number of precision 2 (assuming RoundingMode.HALF_EVEN). By the 
same token, the product 2.9 * 4 produces 12, which is 11.6 rounded to 2 digits.
But really, the position of the decimal point is floating.

IEEE decimals and SQL decimals are fundamentally different and ave different 
arithmetic, so I wouldn't recommend using the proposed classes for JDBC.

On the positive side, SQL decimals, are easier to implement if the maximum 
allowed p in DECIMAL(p, s) is reasonable, say 38. But that's another topic.


Greetings
Raffaello

Re: Faster double parser?

[Raffaello Giulietti]

Hi,

if there's revived interest I would be glad to migrate the code to 
Skara/GitHub for a PR. The last uploaded version is still on Mercurial, 
I assume, under Brian Burkhalter's webrev folder.


But I'd also like to point out that there is a pending CSR as well.

(Sorry for the initiator of this thread: you're speaking about a parser, 
we about the opposite.)



Greetings
Raffaello




The mailing list mangled formatting. My comments are prefixed below by “->”.

On Apr 5, 2021, at 9:52 AM, Brian Burkhalter mailto:brian.burkhalter at oracle.com>> wrote:

On Apr 5, 2021, at 4:11 AM, Andrew Haley mailto:aph at 
redhat.com><mailto:aph at redhat.com>> wrote:

On 4/5/21 9:08 AM, Alan Bateman wrote:
On 04/04/2021 09:53, Andrew Haley wrote:
:
We also have a candidate for a String -> toDouble parser from Raffaello
Giulietti:

https://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/052355.html

As far as I can see there's nothing wrong with it, but it got stuck in
review. This was a bad failure of our review processes, I'm afraid. I
wonder if we will do any better with this one.

Yeah, there was a lengthy discussion about this at the OpenJDK Committer
Workshop just after FOSDEM 2020. Brian Burkhalter and Andrew Dinn had
both put time into looking at the proposal it but the conclusion was
that Raffaello's paper "The Schubfach way to render doubles" needed a
review from from an expert in this area.

-> I spent some time on it a while back but got bogged down.

I wonder. The implementation has even been proved correct for floats by
exhaustion. (Of course that's not possible for doubles.)

-> I exhaustively verified it for floats. I also ran round-trip tests for 
doubles which ran for days without error. Of course one can only disprove by 
counterexample, not prove by anecdote.

I submit, therefore, that we could at least use the float version without
reasonable fear of breaking something.

-> That is a good suggestion.

We seem to be paralysed by this one.

Guy Steele was mentioned but I don't know if he was approached about it.

-> Indirectly.

-> Another aspect which was raised was supportability: is it sufficiently 
understood for us to support? On that note I would ask “Is FloatingDecimal 
sufficiently understood to support?” I think the answer to both questions is the 
same.

Re: Faster double parser?

[Raffaello Giulietti]

Hi,

yes, the code in itself is current.

I would need to adapt it to OpenJDK coding conventions; in particular 
the form of multiline comments is not the usual one. Only cosmetic 
changes, though. Nothing would affect the executable code.


Also, the current patch modifies some existing files, so I need to check 
that nothing has changed in the meantime.


I'll prepare a PR in the next couple of days.




On 2021-04-07 00:44, Brian Burkhalter wrote:


On Apr 6, 2021, at 3:40 PM, Raffaello Giulietti 
mailto:raffaello.giulie...@gmail.com>> 
wrote:


if there's revived interest I would be glad to migrate the code to 
Skara/GitHub for a PR. The last uploaded version is still on 
Mercurial, I assume, under Brian Burkhalter's webrev folder.


Is the Hg version current?

http://cr.openjdk.java.net/~bpb/4511638/webrev.04/ 
<http://cr.openjdk.java.net/~bpb/4511638/webrev.04/>


Converting the patch to Git is straightforward.


But I'd also like to point out that there is a pending CSR as well.


https://bugs.openjdk.java.net/browse/JDK-8202555 
<https://bugs.openjdk.java.net/browse/JDK-8202555>


Thanks,

Brian

Re: RFR: 8264514: HexFormat implementation tweaks [v2]

[Raffaello Giulietti]

> Please review these small tweaks.
> 
> For background information see 
> [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html)
> 
> Greetings
> Raffaello

Raffaello Giulietti has refreshed the contents of this pull request, and 
previous commits have been removed. The incremental views will show differences 
compared to the previous content of the PR.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3295/files
  - new: https://git.openjdk.java.net/jdk/pull/3295/files/123f3101..21e7402b

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

  Stats: 67 lines in 1 file changed: 26 ins; 10 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3295.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3295/head:pull/3295

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

Withdrawn: 8264514: HexFormat implementation tweaks

[Raffaello Giulietti]

On Wed, 31 Mar 2021 21:39:46 GMT, Raffaello Giulietti 
 wrote:

> Please review these small tweaks.
> 
> For background information see 
> [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html)
> 
> Greetings
> Raffaello

This pull request has been closed without being integrated.

-

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

RFR: 8264514: HexFormat implementation tweaks

[Raffaello Giulietti]

(Changed to new branch in personal fork)

Please review these small tweaks.
For background information see 
[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html)

Greetings
Raffaello

-

Commit messages:
 - 8264514: HexFormat implementation tweaks

Changes: https://git.openjdk.java.net/jdk/pull/3381/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3381&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264514
  Stats: 67 lines in 1 file changed: 10 ins; 26 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3381.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3381/head:pull/3381

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

Re: RFR: 8264514: HexFormat implementation tweaks

[Raffaello Giulietti]

On Wed, 7 Apr 2021 18:49:36 GMT, Raffaello Giulietti 
 wrote:

> (Changed to new branch in personal fork)
> 
> Please review these small tweaks.
> For background information see 
> [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html)
> 
> Greetings
> Raffaello

Automatically started Windows x64 tier1 tests are still running after more than 
24 hours...

-

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

RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hello,

here's a PR for a patch submitted on March 2020 
[1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
thing.

The patch has been edited to adhere to OpenJDK code conventions about 
multi-line (block) comments. Nothing in the code proper has changed, except for 
the addition of redundant but clarifying parentheses in some expressions.


Greetings
Raffaello

-

Commit messages:
 - 4511638: Double.toString(double) sometimes produces incorrect results

Changes: https://git.openjdk.java.net/jdk/pull/3402/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3402&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-4511638
  Stats: 4237 lines in 15 files changed: 4116 ins; 54 del; 67 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3402.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Forgot to add that other changes in the code are the use of switch expressions 
and the use of instanceof patterns.

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:20:34 GMT, Raffaello Giulietti 
 wrote:

>> Hello,
>> 
>> here's a PR for a patch submitted on March 2020 
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was 
>> a thing.
>> 
>> The patch has been edited to adhere to OpenJDK code conventions about 
>> multi-line (block) comments. Nothing in the code proper has changed, except 
>> for the addition of redundant but clarifying parentheses in some expressions.
>> 
>> 
>> Greetings
>> Raffaello
>
> Forgot to add that other changes in the code are the use of switch 
> expressions and the use of instanceof patterns.

Hello,

here's some background information for those that didn't follow the mailing 
list for the last couple of years.

Some enjoyable properties of the novel algorithm:
* No intermediate objects are instantiated.
* Loop-free core algorithm.
* Only int and long arithmetic.
* No divisions at all.
* 17.7x speedup (jmh) 
[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html).
* Randomized, yet reproducible deep diving tests (jtreg).
* Clear, unambiguous spec.
* All floats have been tested to fully meet the spec.
* Fully documented in 
[2](https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) 
and/or in comments.

See 
[3](https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062580.html)
 for some (invented) Q&A. The last Q&A deals with your investment in time for 
an informed review.


Greetings
Raffaello

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Fri, 9 Apr 2021 16:39:45 GMT, Brian Burkhalter  wrote:

>> Hello,
>> 
>> here's some background information for those that didn't follow the mailing 
>> list for the last couple of years.
>> 
>> Some enjoyable properties of the novel algorithm:
>> * No intermediate objects are instantiated.
>> * Loop-free core algorithm.
>> * Only int and long arithmetic.
>> * No divisions at all.
>> * 17.7x speedup (jmh) 
>> [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-April/065921.html).
>> * Randomized, yet reproducible deep diving tests (jtreg).
>> * Clear, unambiguous spec.
>> * All floats have been tested to fully meet the spec.
>> * Fully documented in 
>> [2](https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view) 
>> and/or in comments.
>> 
>> See 
>> [3](https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062580.html)
>>  for some (invented) Q&A. The last Q&A deals with your investment in time 
>> for an informed review.
>> 
>> 
>> Greetings
>> Raffaello
>
> @rgiulietti Please issue the `/csr/` command here [1]. Speaking of which, 
> does the CSR [2] need to be updated?
> 
> [1] 
> https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/csr
> [2] https://bugs.openjdk.java.net/browse/JDK-8202555

@bplb No, the CSR [1] (https://bugs.openjdk.java.net/browse/JDK-8202555) needs 
no updates.

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Fri, 9 Apr 2021 16:44:18 GMT, Raffaello Giulietti 
 wrote:

>> @rgiulietti Please issue the `/csr/` command here [1]. Speaking of which, 
>> does the CSR [2] need to be updated?
>> 
>> [1] 
>> https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/csr
>> [2] https://bugs.openjdk.java.net/browse/JDK-8202555
>
> @bplb No, the CSR [1] (https://bugs.openjdk.java.net/browse/JDK-8202555) 
> needs no updates.

The langtools/tier1 test tools/javac/sym/ElementStructureTest.java fails on all 
4 platforms supported by the automatic GH actions.
Does anybody have a clue? Is this something I should be worried about?

Thanks

-

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

Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}

[Raffaello Giulietti]

Hi Jie,

the behavior you report is the one specified by the standard IEEE 754. 
Java follows this standard as closely as it can.


The standard says that
* squareRoot(-0) = -0
* squareRoot(-∞) = NaN

Also, the standard has a long lists of special cases for pow(x, y), 
among them:

* pow(±0, y) is +0 for finite y > 0 and not an odd integer
* pow(-∞, y) is +∞ for finite y > 0 and not an odd integer

Thus, the conflicts you observe originate in following the standard, not 
by special Java rules.


Unfortunately, the IEEE standard does not explain the reasons for the 
special rules. Some are obvious, some are not.



HTH
Raffaello



Hi all,

I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different values as the 
following:
```
Math.pow(-0.0, 0.5) = 0.0
Math.sqrt(-0.0) = -0.0

Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
```

The reason is that both of pow and sqrt have special rules for these 
computations.
For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 0.0.
And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity and 
Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.

I think most people will be confused by these rules because from the view of 
mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).

So why Java creates conflict special rules for them?
Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?

I came across this problem when I was trying to optimize pow(x, 0.5) with 
sqrt(x).
If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), then pow(x, 
0.5)'s performance can be improved by 7x~14x [3].

Thanks.
Best regards,
Jie

Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}(Internet mail)

[Raffaello Giulietti]

Hi Jie,

I don't think that changing the spec of Math.pow() to be misaligned with 
IEEE 754 would be a wise option. IEEE is much more pervasive than Java. 
There are many aspects in IEEE that might be seen as questionable, but 
at least it is a widely adopted standard.


AFAIU, the only reason you would like to "optimize" the special case of 
y = 0.5 in pow(x, y) to return sqrt(x) is for performance, more accuracy 
and some kind of consistency.


But then, why not a special case for y = 0.25 as sqrt(sqrt(x))?
And what about y = 0.75? Should this be translated to sqrt(sqrt(pow(x, 3)))?
What about y = 1.0 / 3.0? Should this become cbrt(x)?
And why not consider y = 2.0 / 3.0 in a special rule: cbrt(x * x)?

You see, the special cases can quickly become unmanageable. Also, 
special rules would produce results which are "discontinuous" with 
nearby exponents, like y = 0.5001.


That's probably why IEEE doesn't propose translation rules for finite 
numerical exponents that are not integers, except when x is a special value.



Greetings
Raffaello



On 2021-04-12 13:44, jiefu(傅杰) wrote:

Hi Andrew H, Andrew D, and Raffaello,

Thank you all for your kind reply and helpful comments.

Now I got where the rules come from.
But I don't think the IEEE standars are reasonable to specify conflits rules.
Maybe, these computations should be open to be implementation dependent.

(If it's possible) I really hope the special cases of Math.pow(x, 0.5) can be 
aligned with Math.sqrt(x) in Java.
We already allow some plausible behaviors to be different with the IEEE 
recommendations for some special cases, right?
And in that case, we can replace pow(x, 0.5) with sqrt(x) safely.

Thanks.
Best regards,
Jie


On 2021/4/12, 6:40 PM, "Raffaello Giulietti"  
wrote:

 Hi Jie,
 
 the behavior you report is the one specified by the standard IEEE 754.

 Java follows this standard as closely as it can.
 
 The standard says that

 * squareRoot(-0) = -0
 * squareRoot(-∞) = NaN
 
 Also, the standard has a long lists of special cases for pow(x, y),

 among them:
 * pow(±0, y) is +0 for finite y > 0 and not an odd integer
 * pow(-∞, y) is +∞ for finite y > 0 and not an odd integer
 
 Thus, the conflicts you observe originate in following the standard, not

 by special Java rules.
 
 Unfortunately, the IEEE standard does not explain the reasons for the

 special rules. Some are obvious, some are not.
 
 
 HTH

 Raffaello
 
 
 > Hi all,

 >
 > I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different values 
as the following:
 > ```
 > Math.pow(-0.0, 0.5) = 0.0
 > Math.sqrt(-0.0) = -0.0
 >
 > Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
 > Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
 > ```
 >
 > The reason is that both of pow and sqrt have special rules for these 
computations.
 > For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 0.0.
 > And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
 > And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = 
Infinity and Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.
 >
 > I think most people will be confused by these rules because from the 
view of mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).
 >
 > So why Java creates conflict special rules for them?
 > Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?
 >
 > I came across this problem when I was trying to optimize pow(x, 0.5) 
with sqrt(x).
 > If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), then 
pow(x, 0.5)'s performance can be improved by 7x~14x [3].
 >
 > Thanks.
 > Best regards,
 > Jie
 
 

What is the status of JEP draft: "Predictable regex performance"?

[Raffaello Giulietti]

Hello,

I'm just curious about the status of the important project described by 
"JEP draft: Predictable regex performance" [1].


Afaiu, there are mentions about:
* Using the native re2 library [2] via JNI, with the cons that usage 
becomes kind of unnatural, like "closing" the pattern to release native 
resources.

* Possibly porting re2 to Java, but it seems rather big, several kLOC.
* Adopting/adapting the incomplete and unmaintained re2j [3] Java 
implementation of re2.
* Using the sregex project [4], but it's unclear to me what makes it 
substantially different from re2.


Less clear is whether the project is still exploring various 
alternatives in addition to the above ones or has started to converge 
towards a preferred choice.



Greetings
Raffaello



[1] https://openjdk.java.net/jeps/8260688
[2] https://github.com/google/re2
[3] https://github.com/rapid7/re2-java
[4] https://github.com/openresty/sregex

Re: ObjectMethods seems generating wrong methods for array-type field

[Raffaello Giulietti]

Hi Kengo,

I don't think this is a bug, as records are designed to "act as 
transparent carriers for immutable data". A record having an array-typed 
component is far from being immutable because everyone can change the 
array contents. So you probably shouldn't opt for records for such use 
cases, since you would loose the semantic benefits of records.


But if you need deeper hashing and comparison over array-typed 
components of records, as you are probably expecting in your example, 
you have to override the record hashCode() and equals() yourself. But 
then (http://openjdk.java.net/jeps/395):
"Any explicit implementation of accessors or the equals or hashCode 
methods should be careful to preserve the semantic invariants of the 
record class."



Greetings
Raffaello




Hello there,


I'm Kengo TODA, an engineer learning about the Record feature.
Today I found a nonintentional behavior, and it seems that the bug database
has no info about it.
Let me ask here to confirm it's by-design or not. If it's a bug, I want to
try to send a patch to OpenJDK.

Here is the code reproducing the nonintentional behavior:
https://gist.github.com/KengoTODA/4d7ef6a5226d347ad9385241fee6bc63

In short, the ObjectMethods class in OpenJDK v16 generates code
that invokes the fields' method, even when the field is an array.

Please help me to understand this behavior, or
make an entry in the bug database to propose a patch.
Thanks in advance! :)

***
Kengo TODA
skypencil at gmail.com

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Hi Jan,

I had to change a string in test
test/jdk/java/lang/String/concat/ImplicitStringConcatBoundaries.java
because it failed with the current string but passes with the new one. Indeed, 
the new implementation of Float.toString(float) produces the new string, which, 
like the current one, is correct in the sense that, upon reading, it recovers 
Float.MIN_NORMAL.

However, I didn't change the definition of MIN_NORMAL in java.lang.Float 
because there it is already expressed in hex notation.

As suggested before and by Joe, using the hex representation instead of the 
decimal would be more robust because the conversions from/to hex are almost 
trivial, hence much less subject to slight errors. So, rather than printing the 
raw bits as you suggest, you could use the hex string rendering instead.


Thanks
Raffaello

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Fine, thanks!

Will your change be integrated soon on master?
What am I supposed to do then, rebasing my branch with the updated master?

(BTW, you could make use of instanceof pattern matching if you don't need 
backward compat ;-) )

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Fri, 16 Apr 2021 08:22:41 GMT, Jan Lahoda  wrote:

>> Hello,
>> 
>> here's a PR for a patch submitted on March 2020 
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was 
>> a thing.
>> 
>> The patch has been edited to adhere to OpenJDK code conventions about 
>> multi-line (block) comments. Nothing in the code proper has changed, except 
>> for the addition of redundant but clarifying parentheses in some expressions.
>> 
>> 
>> Greetings
>> Raffaello
>
> @ rgiulietti, I thought you'd simply add the test change to your patch. But I 
> can start a PR for it, if you prefer.

@lahodaj Oh, my understanding was that yours is a permanent change worth of 
integrating anyway. But yes, I can add your change to my changeset.

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]

[Raffaello Giulietti]

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Raffaello Giulietti has updated the pull request incrementally with one 
additional commit since the last revision:

  4511638: Double.toString(double) sometimes produces incorrect results

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3402/files
  - new: https://git.openjdk.java.net/jdk/pull/3402/files/cc10a92d..22092f0c

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

  Stats: 26 lines in 1 file changed: 10 ins; 7 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3402.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Modified
test/langtools/tools/javac/sym/ElementStructureTest.java
as suggested by @lahodaj

-

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