Re: RFR: 8197594 - String and character repeat

2018-02-15 Thread Louis Wasserman
I don't think there's a case for demand to merit having a
repeat(CharSequence, int) at all.

I did an analysis of usages of Guava's Strings.repeat on Google's
codebase.  Users might be rolling their own implementations, too, but this
should be a very good proxy for demand.

StringRepeat_SingleConstantChar = 4.475 K // strings with .length() ==  1
StringRepeat_SingleConstantCodePoint = 28 // strings with
.codePointCount(...) == 1
StringRepeat_MultiCodePointConstant = 1.156 K // constant strings neither
of the above
StringRepeat_CharSequenceToString = 2 //
Strings.repeat(CharSequence.toString(), n)
StringRepeat_NoneOfTheAbove = 248

Notably, it seems like basically nobody needs to repeat a CharSequence --
definitely not enough demand to merit the awkwardness of e.g.
Rope.repeat(n) inheriting a repeat returning a String.

Based on this data, I'd recommend providing one and only one method of this
type: String.repeat(int).  There's no real advantage to a static
repeat(char, int) method when the overwhelming majority of these are
constants: e.g. compare SomeUtilClass.repeat('*', n) versus "*".repeat(n).
Character.toString(c).repeat(n) isn't a bad workaround if you don't have a
constant char.  There also isn't much demand for dealing with the code
point case specially; the String.repeat(int) method seems like it'd handle
that just fine.

On Thu, Feb 15, 2018 at 11:44 AM Jim Laskey  wrote:

>
>
> > On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov 
> wrote:
> >
> > Hello!
> >
> > The link with the webrev returned 404, but I could find it at this
> location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
> >
> > A few minor comments:
> >
> > 1)
> >
> > This check:
> >
> > 2992 final long limit = (long)count * 2L;
> > 2993 if ((long)Integer.MAX_VALUE < limit) {
> >
> > can be possibly simplified as
> > if (count > Integer.MAX_VALUE - count) {
>
> Good.
>
> >
> > 2)
> > Should String repeat(final int codepoint, final int count) be optimized
> for codepoints that can be represented with a single char?
> >
> > E.g. like this:
> >
> > public static String repeat(final int codepoint, final int count) {
> >return Character.isBmpCodePoint(codepoint))
> >? repeat((char) codepoint, count)
> >: (new String(Character.toChars(codepoint))).repeat(count);
> > }
>
> Yes, avoid array allocation.
>
> >
> > 3)
> > Using long arithmetic can possibly be avoided in the common path of
> repeat(final int count):
> >
> > E.g. like this:
> >
> > if (count < 0) {
> > throw new IllegalArgumentException("count is negative, " +
> count);
> > } else if (count == 1) {
> > return this;
> > } else if (count == 0) {
> > return "";
> > }
> > final int len = value.length;
> > if (Integer.MAX_VALUE / count < len) {
> > throw new IllegalArgumentException(
> > "Resulting string exceeds maximum string length: " +
> ((long)len * (long)count));
> > }
> > final int limit = count * len;
>
> Good.
>
> Thank you.
>
> >
> > With kind regards,
> > Ivan
> >
> > On 2/15/18 9:20 AM, Jim Laskey wrote:
> >> This is a pre-CSR code review [1] for String repeat methods
> (Enhancement).
> >>
> >> The proposal is to introduce four new methods;
> >>
> >> 1. public String repeat(final int count)
> >> 2. public static String repeat(final char ch, final int count)
> >> 3. public static String repeat(final int codepoint, final int count)
> >> 4. public static String repeat(final CharSequence seq, final int count)
> >>
> >> For the sake of transparency, only 1 is necessary, 2-4 are convenience
> methods.
> >> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’,
> 10).
> >> 3 and 4 convert to String before calling 1.
> >>
> >> Performance runs with jmh (results as comment in [2]) show that these
> >> methods are significantly faster that StringBuilder equivalents.
> >>  - fewer memory allocations
> >>  - fewer char to byte array conversions
> >>  - faster pyramid replication vs O(N) copying
> >>
> >> I left StringBuilder out of scope. It falls under the category of
> >> Appendables#append with repeat. A much bigger project.
> >>
> >> All comments welcome. Especially around the need for convenience
> >> methods, the JavaDoc content and expanding the tests.
> >>
> >> — Jim
> >>
> >> [1] webrev:
> http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
> >> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
> >>
> >>
> >
> > --
> > With kind regards,
> > Ivan Gerasimov
> >
>
>


Re: RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

2017-11-01 Thread Louis Wasserman
I disagree, actually.  Collections with size zero and one are significantly
more common than bigger collections.

In Guava's immutable collection factories (ImmutableList.of(...) etc.), we
observed a roughly exponential decline in the number of users of factory
methods of each size: if N people created empty lists, ~N/2 created
singleton lists, ~N/4 created two-element lists, etc.  We got noticeable
pushback from RAM-sensitive customers when we eliminated some specialized
singleton collection implementations.

Our experience has been that specializing for N=0 and N=1 does pay off.
Probably not N=2, though?

On Wed, Nov 1, 2017 at 12:45 PM Patrick Reinhart  wrote:

>
> > Am 01.11.2017 um 20:25 schrieb Stuart Marks :
> >
> > On 10/31/17 5:52 PM, Bernd Eckenfels wrote:
> >> Having a List.of(List) copy constructor would save an additional array
> copy in the collector Which uses  (List)List.of(list.toArray())
> >
> > The quickest way to get all the elements from the source collection is
> to call toArray() on it. Some copy constructors (like ArrayList's) simply
> use the returned array for internal storage. This saves a copy, but it
> relies on the source collection's toArray() implementation to be correct.
> In particular, the returned array must be freshly created, and that array
> must be of type Object[]. If either of these is violated, it will break the
> ArrayList.
> >
> > The "immutable" collections behind List.of/copyOf/etc. are fastidious
> about allocating their internal arrays in order to ensure that they are
> referenced only from within the newly created collection. This requires
> making an „extra" copy of the array returned by toArray().
> >
> > An alternative is for the new collection to preallocate its internal
> array using the source's size(), and then to copy the elements out. But the
> source’s
> > size might change during the copy (e.g., if it’s a concurrent
> collection) so this complicates things.
>
> I think the array „overhead“ would be only for the cases of zero, one and
> two value implementations. That seems to me not worth of optimizing…
>
> > I think the only safe way to avoid the extra copy is to create a private
> interface that can be used by JDK implementations.
> >
> > s'marks
>
> -Patrick
>


Re: RFR: 8166365: Small immutable collections should provide optimized implementations when possible

2017-01-11 Thread Louis Wasserman
I haven't followed this much, but an observation: in Guava, we avoided
creating lots of specialized implementations for small collections, because
the JVM, at least at the time, had a sweet spot for bimorphic dispatch:
method calls where the real implementation would be one of two options, and
that the cost blew up after you hit three, and we considered hitting that
sweet spot more worthwhile than the slightly smaller overhead for
collections that were already small.

As a result, many of our immutable collection implementations converged on
having two implementations: one single-element implementation, and one
implementation for everything else (0, 2, 3... but not 1), and then we had
a singleton static constant object for the 0-element case.

I don't know if that calculus has changed, but it seemed worth mentioning.

On Wed, Jan 11, 2017 at 10:06 AM Martin Buchholz 
wrote:

> On Wed, Jan 11, 2017 at 5:43 AM, Claes Redestad  >
> wrote:
>
> >
> >> ---
> >>
> >> In Set2, SetN, Map1, and MapN, the addition of @Stable also dropped the
> >> "private" access modifier. Other implementations still have
> corresponding
> >> fields as private. Was this intentional? I think they should all remain
> >> private.
> >>
> >
> > This was intentional: for those implementations where I dropped private
> > there are inner
> > classes which access the fields directly.  This leads to synthetic
> > accessors, which shows up
> > as JIT compilation overhead during module bootstrap profiling.
> >
>
> In java.util.concurrent, our policy is to avoid creating synthetic
> accessors, so we also promote private to package private when accessed by
> nested classes, and this seems to be the best engineering compromise...
> (until we get nestmates?)
>
>
> > This is what made me look at this in the first place, and a large part of
> > the reason why I
> > believe that it's motivated to fast-track this enhancement as a means to
> > improve JDK 9
> > startup metrics.  If you have a strong preference against them being made
> > package-private
> > there's a similar gain to be had by passing them as arguments to concrete
> > inner classes,
> > e.g., for Set2:
> >
> > static class Itr implements Iterator {
> > private final E e0;
> > private final E e1;
> > private int idx = 0;
> > Itr(E e0, E e1) {
> > this.e0 = e0;
> > this.e1 = e1;
> > }
> > @Override
> > public boolean hasNext() {
> > return idx < 2;
> > }
> >
> > @Override
> > public E next() {
> > if (idx == 0) {
> > idx = 1;
> > return e0;
> > } else if (idx == 1) {
> > idx = 2;
> > return e1;
> > } else {
> > throw new NoSuchElementException();
> > }
> > }
> > }
> >
> > @Override
> > public Iterator iterator() {
> > return new Itr(e0, e1);
> > }
> >
>


Re: ParallelStream Vs Stream Digest, Vol 113, Issue 94

2016-09-29 Thread Louis Wasserman
You should absolutely not assume parallel streams are faster than
sequential streams.
http://gee.cs.oswego.edu/dl/html/StreamParallelGuidance.html is pretty much
the iconic document on that subject, and explains circumstances under which
parallelism is good, and when it is likely to be harmful.

On Thu, Sep 29, 2016 at 10:07 PM Prakhar Makhija  wrote:

> The application makes a hit to a core object over and over again. I have to
> copy this object, i.e. make a clone of it using the Cloneable interface, so
> that the original object cannot be modified. But since the references of
> the old object and clone object would be intact, inside the clone method I
> am explicitly copying the List Map and Set using parrallelStream/stream.
>
> The hardware is i3 processor with 8GB RAM and 1TB hard disk.
>
> So you mean to say, Parallel Stream is good for large data set?
> On Sep 30, 2016 10:08 AM, "David Holmes"  wrote:
>
> > On 30/09/2016 2:24 PM, Prakhar Makhija wrote:
> >
> >> Hi everyone,
> >>
> >> I have started using both Stream and ParallelStream, for Set List and
> >> Entry
> >> of Map.
> >>
> >> What I can't understand is why Stream is taking lesser time than
> >> ParallelStream.
> >>
> >> Shouldnt ParallelStream be giving better performance than Stream in
> terms
> >> of Time Complexity?
> >>
> >
> > Depends on the data set size and your hardware, and what exactly you are
> > trying to do in parallel.
> >
> > David
> >
>


Re: Make iterators cloneable?

2016-09-10 Thread Louis Wasserman
Some iterators might be.  Many may not be.  Certainly Iterator as an
interface has been out there for long enough there are Iterator
implementations out there that aren't cloneable -- say, Iterators reading
from a BufferedReader, where there really won't be any way to do what
you're hoping for; BufferedReaders certainly aren't cloneable.

On Sat, Sep 10, 2016 at 4:33 PM Dave Brosius 
wrote:

> Yes Louis is correct.
>
> I want the pair wise associations or all elements of a set.
>
> Fee-Fi
>
> Fee-Fo
>
> Fee-Fum
>
> Fi-Fo
>
> Fi-Fum
>
> Fo-Fum
>
>
> the independent iterators produce Fee-Fee (etc) as well as the duplicate
> Fee-Fi and Fi-Fee (etc), both of which i don't want.
>
>
> This is obviously simplistic with index based collections, but not with
> sets/maps
>
> I don't see why an Iterator isn't by nature easily cloneable.
>
>
>
> On 09/10/2016 06:45 PM, Jonathan Bluett-Duncan wrote:
>
> Ah okay Louis, if that's the case then that certainly makes sense, and I'd
> agree that there's no good way of doing so, as one would need to copy the
> set into a list.
>
> Dave, did Louis hit the mark? If not, would you kindly go into further
> detail as to exactly what it is you're trying to do?
>
> Best,
> Jonathan
>
> On 10 September 2016 at 23:36, Jonathan Bluett-Duncan <
> jbluettdun...@gmail.com> wrote:
>
> Hi Dave,
>
> Rather than using Iterator.clone(), how about you just call
> collection.iterator() 2 times to return 2 unique, non-same iterators;
> something like the following:
>
> import java.util.Collections;import java.util.Iterator;import 
> java.util.Set;import java.util.concurrent.ConcurrentHashMap;
> public class Example {
>   public static void main(String[] args) {
> Set s = Collections.newSetFromMap(new ConcurrentHashMap Boolean>());
>
> s.add("Fee");
> s.add("Fi");
> s.add("Fo");
> s.add("Fum");
>
> Iterator it1 = s.iterator();for (String v1 = null; 
> it1.hasNext(); v1 =it1.next()) {
>   Iterator it2 = s.iterator(); // a completely separate iterator 
> to it1  for (String v2 = null; it2.hasNext(); v2 = it2.next()) {
> System.out.println(v1 + " <-->" + v2);
>   }
> }
>   }
> }
>
> Or, even better, if you're using Java 5+, you can skip using Iterators
> altogether and use for-loops directly:
>
> import java.util.Collections;import java.util.Set;import 
> java.util.concurrent.ConcurrentHashMap;
> public class Example {
>   public static void main(String[] args) {
> Set s = Collections.newSetFromMap(new ConcurrentHashMap Boolean>());
>
> s.add("Fee");
> s.add("Fi");
> s.add("Fo");
> s.add("Fum");
> for (String v1 : s) {
>   for (String v2 : s) {
> System.out.println(v1 + "<-->" + v2);
>   }
> }
>   }
> }
>
> Kind regards,
> Jonathan
> On 10 September 2016 at 23:13, Dave Brosius 
> wrote:
>
> It would be nice to be able to associate each element in a collection with
> another element in the collection, which is something very easily done with
> index based collections, but with sets, etc this isn't so easy... unless
> i'm having a brainfart. So i'd like to do this, but Iterator doesn't
> implement Cloneable... Any reason not to? or is there another way that's
> missing me? public class ItClone { public static void main(String[]
> args) { Set s = Collections.newSetFromMap(new
> ConcurrentHashMap()); s.add("Fee");
> s.add("Fi"); s.add("Fo"); s.add("Fum");
> Iterator it1 = s.iterator(); while (it1.hasNext()) {
>   String v1 = it1.next(); Iterator it2 =
> (Iterator) it1.*clone*(); while (it2.hasNext()) {
>   String v2 = it2.next(); System.out.println(v1 + "
> <-->" + v2); } } } }
>
>


Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-05 Thread Louis Wasserman
Where is the current commitment to unspecified iteration order?  Is that in
Java 9?

Generally speaking, I see no problem with going from unspecified to
specified iteration order; if code had to be able to deal with *any* order
previously they can certainly deal with an order that happens to be the
obvious sensible order.

Furthermore, the code that is easiest to use should be the simplest, most
desirable use case, and I'd definitely say that means using the predictable
order the elements were provided in.  Providing that as another, more
awkwardly named method seems deeply undesirable: Set.of and Map.of are the
short, easy to use method names, and they should have the simplest,
easiest-to-understand behavior: insertion order iteration.

On Thu, May 5, 2016 at 9:27 AM Peter Levart  wrote:

> Hi,
>
>
> On 05/05/2016 02:54 PM, Stephen Colebourne wrote:
> > On 5 May 2016 at 02:30, Stuart Marks  wrote:
> >>> I disagree with altering the iteration order. Guava's ImmutableSet and
> >>> ImmutableMap have reliable iteration specified to match creation
> >>> order. This aspect of the design is very useful.
> >>
> >> I think this is a reasonable thing to want, but it would be a different
> API.
> >> The current Set.of() and Map.of() APIs have unspecified iteration
> order, and
> >> I want to "enforce" that via randomizing the iteration order. Having
> >> unspecified iteration order, but with the implementation giving a stable
> >> order, is the worst of both worlds. It lets clients bake in inadvertent
> >> order dependencies, and then when we do change it, it breaks them. (This
> >> seems to happen every couple years with HashMap.) Randomizing iteration
> >> order helps preserve implementation freedom.
> >>
> >> That said, it's a fine thing to have a different API that gives a Set
> or Map
> >> with a stable iteration order. Several people have asked for this. I've
> >> filed JDK-8156070 to track this.
> > To be clear, I believe that the spec of Set.of() and Map.of() should
> > require iteration order matching insertion order (because the order is
> > very clearly defined in this case and anything else will be
> > surprising).
> >
> > Stephen
>
> So what if the API was specified to have iteration order matching
> construction order. Are you afraid that would impact implementations so
> that they would be less compact or less performant? At least for small
> number of elements, there is a possible implementation that keeps
> construction order and is even more compact and still has O(1) lookup
> performance. For example an implementation for up to 255 elements:
>
> public class Set0xFF extends AbstractSet implements Serializable {
>
>  /**
>   * A "salt" value used for randomizing iteration order. This is
> initialized once
>   * and stays constant for the lifetime of the JVM. It need not be
> truly random, but
>   * it needs to vary sufficiently from one run to the next so that
> iteration order
>   * will vary between JVM runs.
>   */
>  static final int SALT;
>
>  static {
>  SALT = new Random().nextInt();
>  }
>
>  /**
>   * The reciprocal of load factor. Given a number of elements
>   * to store, multiply by this factor to get the table size.
>   */
>  static final double EXPAND_FACTOR = 2.0;
>
>  final E[] elements;
>  final byte[] index;
>
>  @SafeVarargs
>  @SuppressWarnings("unchecked")
>  Set0xFF(E... input) {
>  if (input.length > 255) {
>  throw new IllegalArgumentException("To many elements for
> this implementation");
>  }
>  elements = input.clone();
>  index = new byte[(int) Math.ceil(EXPAND_FACTOR *
> elements.length)];
>  for (int i = 0; i < elements.length; i++) {
>  E e = Objects.requireNonNull(elements[i]);
>  int idx = probe(e);
>  if (idx >= 0) {
>  throw new IllegalArgumentException("duplicate element:
> " + e);
>  } else {
>  index[-(idx + 1)] = (byte) (i + 1);
>  }
>  }
>  }
>
>  @Override
>  public int size() {
>  return elements.length;
>  }
>
>  @Override
>  @SuppressWarnings("unchecked")
>  public boolean contains(Object o) {
>  Objects.requireNonNull(o);
>  return probe((E) o) >= 0;
>  }
>
>  @Override
>  public Iterator iterator() {
>  return new Iterator() {
>  int idx = 0;
>
>  @Override
>  public boolean hasNext() {
>  return idx < elements.length;
>  }
>
>  @Override
>  public E next() {
>  int i = idx;
>  if (i >= elements.length) {
>  throw new NoSuchElementException();
>  }
>  idx = i + 1;
>  return elements[i];
>  }
>  };
>  }
>
>  // returns index into index arra

Re: [9] RFR of 8032027: Add BigInteger square root methods

2015-12-09 Thread Louis Wasserman
Guava's tests check the explicit definition of square root (mentioned by
Joe above) on 2^n +/- 1 for all n up to Double.MAX_EXPONENT + 1, because
why not?

On Wed, Dec 9, 2015 at 6:12 PM Joseph D. Darcy  wrote:

> Hi Brian,
>
> New version looks good.
>
> One more case to try: start with a BigInteger that would overflow to
> Double.POSITIVE_INFINITY when the doubleValue method was called. If this
> case doesn't take too long to run, it would be a fine additional case to
> add to the test. 2^1024 should be fine input value. More precisely,
>
>   (new
> BigDecimal(Double.MAX_VALUE)).toBigInteger().add(BigInteger.ONE);
>
> should do the trick. If the code passes with this value, you're okay to
> push. Well, while you're at it, might as well verify
>
>  (new BigDecimal(Double.MAX_VALUE)).toBigInteger()
>
> behaves well too ;-)
>
> Thanks,
>
> -Joe
>
> On 12/9/2015 5:41 PM, Brian Burkhalter wrote:
> > Hi Joe,
> >
> > On Dec 1, 2015, at 7:25 PM, Joseph D. Darcy  > > wrote:
> >
> >> Current version looks okay. One more request, before pushing please
> >> add explicit test cases for the for the largest number having 63 bits
> >> and the smallest number having 64 bits. No need for another round of
> >> webrevs for that.
> >
> > Well there is after all a need for another round of review:
> >
> > http://cr.openjdk.java.net/~bpb/8032027/webrev.04/
> > 
> >
> > That was a good call to add the above tests: one of them failed. This
> > was found to be due to a floor() where there should have been a ceil().
> >
> > Summary:
> >
> > MutableBigInteger: at line 1920 change Math.floor(.) to Math.ceil(.).
> > BigIntegerTest: at lines 331-340 add testing of 2^N and 2^N - 1, 0 < N
> > < 1024
> >
> > Thanks,
> >
> > Brian
>
>


Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-10 Thread Louis Wasserman
I'm unclear on whether the question is where Guava stopped, or why we
included the fixed-args versions as well as the varargs versions?

Part of the answer, of course, is that those factories predate
@SafeVarargs, and frankly even now Guava doesn't really depend on Java 7.

If you're asking about why we stopped where we did, we collected actually
rather a lot of data on the size of static collection constants using
immutable collections, both with builder syntax and without.
https://github.com/google/guava/issues/2071#issuecomment-126468933 has the
statistics (in terms of ratios, not absolute numbers), but we found that
there was right about an exponential trend: static collection constants of
size n+1 were ~half as common as collection constants of size n.

On Sat, Oct 10, 2015 at 6:56 AM Remi Forax  wrote:

> - Mail original -
> > De: "Stephen Colebourne" 
> > À: "core-libs-dev" 
> > Envoyé: Vendredi 9 Octobre 2015 15:11:47
> > Objet: Re: RFC: draft API for JEP 269 Convenience Collection Factories
> >
> > On 9 October 2015 at 00:39, Stuart Marks 
> wrote:
>
> [...]
>
> > > 2. Other concrete collection factories.
> > >
> > > I've chosen to provide factories for the concrete collections
> ArrayList,
> > > HashSet, and HashMap, since those seem to be the most commonly used. Is
> > > there a need to provide factories for other concrete collections, such
> as
> > > LinkedHashMap?
> >
> > LinkedHashMap definitely
> > LinkedList definitely not (as its very slow and use should not be
> > encouraged).
> > TreeSet/TreeMap, maybe, they'd need an extra parameter though.
>
> There is an issue with LinkedHashMap (resp LinkedHashSet),
> it inherits from HashMap /facepalm/, and static methods are accessible
> through class inheritance /facepalm/.
> So if LinkedHashMap doesn't declare some methods of(),
>   LinkedHashMap.of("foo")
> will return a HashMap :(
>
> cheers,
> Rémi
>


Re: [9] RFR of 8032027: Add BigInteger square root methods

2015-10-02 Thread Louis Wasserman
I'm pretty sure we could potentially public-domain our implementation, if
that were an issue.

> The implementation I have here is effectively the one from Hacker’s
Delight (2nd ed.). The initial guess is intended to be larger than the true
result in order to simplify the termination condition of that algorithm as
the monotonic convergence cannot have any oscillation between potential
terminal values.

This isn't a problem.  The arithmetic mean of *any* two nonnegative numbers
is always greater than their geometric mean, so for *any* nonnegative a,
(a + x/a)/2 >= sqrt(a * x/a) = sqrt(x).  So once you do *one* Newton
iteration, the convergence is guaranteed to be monotonically decreasing
after that point.

Newton's method doubles the number of correct digits with every iteration.
So you're paying one extra Newton iteration, but in exchange you're getting
-handwave- 50 correct bits to start out with.  That *more* than pays for
itself.

> There is certainly some room here for improvement in the range of input
values less than Double.MAX_VALUE but this is a first stab at the
implementation so hopefully such improvement may be brought in later if it
is not in the first pass.

It doesn't matter whether the input is bigger than Double.MAX_VALUE.  You
can just find some even number, s, such that x.shiftRight(s) < 2^52.  Then
use

doubleToBigInteger(Math.sqrt(x.shiftRight(s))).shiftLeft(s / 2)

as your starting estimate.  You're still getting ~50 correct bits to start
your Newton iteration.

On Fri, Oct 2, 2015 at 2:08 PM Brian Burkhalter 
wrote:

> I am aware of the classes at
>
>
> http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/math/BigIntegerMath.html
>
> but have not looked at anything beyond the javadoc specification primarily
> in the interest of licensing purity. Perhaps that is not a problem but I
> preferred to stay clear of it for now.
>
> The implementation I have here is effectively the one from Hacker’s
> Delight (2nd ed.). The initial guess is intended to be larger than the true
> result in order to simplify the termination condition of that algorithm as
> the monotonic convergence cannot have any oscillation between potential
> terminal values.
>
> There is certainly some room here for improvement in the range of input
> values less than Double.MAX_VALUE but this is a first stab at the
> implementation so hopefully such improvement may be brought in later if it
> is not in the first pass.
>
> Thanks,
>
> Brian
>
> On Oct 2, 2015, at 1:54 PM, Louis Wasserman  wrote:
>
> > Have you investigated Guava's really quite tightly optimized
> implementation here?
> https://github.com/google/guava/blob/master/guava/src/com/google/common/math/BigIntegerMath.java#L242
> >
> > A few years back I submitted a patch (now applied) to make
> BigInteger.doubleValue() very fast.  If I recall correctly, that patch was
> originally inspired by my attempt to optimize sqrt on BigIntegers at the
> time.
> >
> > Converting the BigInteger to a double, using Math.sqrt(double), and
> using that as a starting point for Newton's method (converting that
> floating point value back to a BigInteger) buys you many bits of precision
> very fast.  That result isn't guaranteed to be >= the true result, but one
> iteration of Newton's method will fix that.
> >
> > I'm quite certain you could improve on Guava's implementation even
> further with access to MutableBigInteger.
>
>


Re: [9] RFR of 8032027: Add BigInteger square root methods

2015-10-02 Thread Louis Wasserman
Have you investigated Guava's really quite tightly optimized implementation
here?
https://github.com/google/guava/blob/master/guava/src/com/google/common/math/BigIntegerMath.java#L242


A few years back I submitted a patch (now applied) to make
BigInteger.doubleValue() very fast.  If I recall correctly, that patch was
originally inspired by my attempt to optimize sqrt on BigIntegers at the
time.

Converting the BigInteger to a double, using Math.sqrt(double), and using
that as a starting point for Newton's method (converting that floating
point value back to a BigInteger) buys you many bits of precision very
fast.  That result isn't guaranteed to be >= the true result, but one
iteration of Newton's method will fix that.

I'm quite certain you could improve on Guava's implementation even further
with access to MutableBigInteger.

On Fri, Oct 2, 2015 at 1:42 PM Brian Burkhalter 
wrote:

> Please review at your convenience.
>
> Issue:  https://bugs.openjdk.java.net/browse/JDK-8032027
> Patch:  http://cr.openjdk.java.net/~bpb/8032027/webrev.00/
>
> Summary: Add sqrt() and sqrtAndRemainder() methods. The square root
> calculated is the integer square root ’s’ such that for a given integer ’n’
> the value of ’s’ is the largest integer such that s*s <= n. In other words
> it is the floor of the mathematical square root of the value ’n' taken as a
> mathematical real number.
>
> The method of calculation is Newton iteration starting from a value larger
> than the square root which ensures that the convergence is monotonically
> decreasing to the result. An effort is made to make the implementation
> efficient in particular with respect to the amount of memory used. Any
> suggestions as to the improvement of the approach concerning memory use or
> any other aspect of the algorithm would be appreciated, as would be
> suggestions regarding the test.
>
> Thanks,
>
> Brian


Re: RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

2015-05-05 Thread Louis Wasserman
Just checking -- IIRC, this will change the semantics of how structural
modifications to a subList of a subList will affect the first subList.  For
example, I believe in the past, removing an element from a subList of a
subList would decrease the size of the first subList by 1, but now the
first subList will represent the same range of indices, and another element
will be moved into that subList.

Is that an accurate representation of current behavior?  Is changing that
behavior acceptable in this context?

On Tue, May 5, 2015 at 11:02 AM Ivan Gerasimov 
wrote:

> Hi Paul
>
> On 05.05.2015 19:56, Paul Sandoz wrote:
> > Hi Ivan,
> >
> > ArrayList
> > --
> >
> > You can simplify SubList with:
> >
> > private final class SubList extends AbstractList implements
> RandomAccess {
> >  private final SubList parent;
> >  private final int offset;
> >  int size;
> >
> >  // Top level sub-list
> >  SubList(int offset, int fromIndex, int toIndex) {
> >  this.parent = null;
> >  this.offset = offset + fromIndex;
> >  this.size = toIndex - fromIndex;
> >  this.modCount = ArrayList.this.modCount;
> >  }
> >
> >  // Sub sub-lst
> >  SubList(SubList parent,
> >  int offset, int fromIndex, int toIndex) {
> >  this.parent = parent;
> >  this.offset = offset + fromIndex;
> >  this.size = toIndex - fromIndex;
> >  this.modCount = ArrayList.this.modCount;
> >  }
> >
> > ArrayList.subList becomes:
> >
> > public List subList(int fromIndex, int toIndex) {
> >  subListRangeCheck(fromIndex, toIndex, size);
> >  return new SubList(0, fromIndex, toIndex);
> > }
> >
> > And SubList.subList:
> >
> > public List subList(int fromIndex, int toIndex) {
> >  subListRangeCheck(fromIndex, toIndex, size);
> >  return new SubList(this, offset, fromIndex, toIndex);
> > }
> >
> > And SubList. updateSizeAndModCount:
> >
> > private void updateSizeAndModCount(int sizeChange) {
> >  int modCount = ArrayList.this.modCount;
> >  for (SubList slist = this; slist != null; slist = slist.parent) {
> >  slist.size += sizeChange;
> >  slist.modCount = modCount;
> >  }
> > }
> >
> Thanks for suggestion!
> I should have realized this myself, that there's no need to set parent
> to ArrayList.this.
> It was a left-over from the previous design, when parent was used in
> different ways.
>
> > AbstractList
> > --
> >
> > Similar changes can be made as above to ArrayList.SubList etc.
> >
> > The construction of sub-lists does indeed require a second take. A
> comment is worthwhile. IMO such scoping is not necessary for ArrayList, i
> have actually found it rare to require such scoping so using it when not
> necessary is rather jarring.
> >
> Okay, I'll reorganize it to make SubList classes stand-alone, not inner
> classes.
> Let's see, if it makes the things nicer.
>
> > NestedSubList
> > --
> >
> > My preference is you use a testng data provider so you don't have to
> roll your own failure checking and reporting.
> > Are there any other tests for testing the integrity of sublists?
>
> I found only a few tests that test some parts of the functionality:
> test/java/util/List/LockStep.java
> test/java/util/Collection/MOAT.java
>
>
> I'll post an update on the code and test soon.
>
> Sincerely yours,
> Ivan
>
>


Re: String concatenation tweaks

2015-04-07 Thread Louis Wasserman
Could that actually be provided immutability-safely?  I suppose an
append-only, fixed-length builder would be potentially safe.

Part of the trickiness there is with primitive parameters, where presizing
and doing the actual append both require calculating the size of the
primitive when converted to a string.  The current approach just uses an
upper bound for the primitive type as a whole, which is fine and allocates
a small constant excess in most cases, but I'm not sure how we could avoid
duplicating that computation in a public API.

On Tue, Apr 7, 2015 at 2:07 PM Peter Levart  wrote:

>  Hi Louis,
>
> This is nice. Amd could be even nicer. In case the estimated initial
> StringBuilder capacity is exactly the final String length, then
> constructing the String could skip the char[] copying step as the
> StringBuilder instance does not escape. But in order to be safe, this would
> have to be a special kind of StringBuilder. Like the following:
>
>
> http://cr.openjdk.java.net/~plevart/misc/ThreadLocalStringBuilder/webrev.01/
>
> Such class would be useful for direct API use too.
>
>
> Regards, Peter
>
>
> On 03/13/2015 10:40 PM, Louis Wasserman wrote:
>
> Got it.  I think the only cases we have to worry about, then, are buffer
> size overflows resulting in NegativeArraySizeException, or possibly an
> explicitly thrown OutOfMemoryError (which is StringBuilder's response when
> the buffer size tries to exceed Integer.MAX_VALUE).  I think we might
> conceivably deal with this by rewriting the bytecode to -- I think we can
> improve on this with jump hackery to avoid repetition, but essentially --
>
>  int length = 3; // sum of the constant strings; we can check that this
> won't overflow at compile time but I think it couldn't no matter what
> because of method length constraints
> String mStr = m().toString();
> length += mStr.length();
> if (length < 0) {
>   throw new OutOfMemoryError();
> }
> String nStr = n().toString();
> length += nStr.length();
> if (length < 0) {
>   throw new OutOfMemoryError();
> }
>
>  This continues to expand the bytecode, but probably manageably -- we
> don't actually need a local for length; if (int < 0) is easy in bytecode,
> and we can have only one OOME site that all the ifs jump to?
>
> On Fri, Mar 13, 2015 at 12:59 PM Alex Buckley 
> wrote:
>
>> I do recognize that the proposed implementation doesn't reorder the
>> evaluation of subexpressions.
>>
>> When discussing the proposed implementation of '+' -- whose key element
>> is calling append(String) with a pre-computed value -- I was careful to
>> set aside asynchronous OOMEs, but I see that even synchronous OOMEs are
>> sidetracking us. My concern is not heap pressure; my concern is
>> arbitrary unchecked exceptions arising from the (int) and
>> append(String) calls.
>>
>> For sake of argument, I'll simplify "unchecked exceptions" to just
>> RuntimeExceptions, not Errors. If you can guarantee that no
>> RuntimeExceptions are thrown synchronously during the execution of those
>> method bodies on the JVM, then '+' cannot fail and the timing of
>> subexpression evaluation is unobservable (ordering is still observable,
>> as required). I think this guarantee is just a matter of reviewing the
>> method bodies.
>>
>> Alex
>>
>> On 3/12/2015 6:01 PM, Louis Wasserman wrote:
>> > I confess I'm not sure how that "quality" goal would be achievable in
>> > bytecode without deliberately allocating arrays we then discard.
>> >
>> > For what it's worth, delaying or avoiding OOMEs seems a desirable goal
>> > in general, and up to a constant multiplicative factor, this
>> > implementation seems to allocate the same amount in the same order.
>> > That is, we're still computing m().toString() before n().toString(), and
>> > up to a constant multiplicative factor, m().toString() allocates the
>> > same number of bytes as the StringBuilder the status quo generates.  So
>> > if m() does something like allocate a char[Integer.MAX_VALUE], we still
>> > OOM at the appropriate time.
>> >
>> > Other notes: this implementation would tend to decrease maximum
>> > allocation, so it'd reduce OOMEs.  Also, since the StringBuilder will
>> > never need further expansion and we're only using the String and
>> > primitive overloads of append, the only way for append to OOME would be
>> > in append(float) and append(double), which allocate a FloatingDecimal
>> > (which may, in turn, allocate a new thread-local char[26] if 

Re: RFR 8071670: java.util.Optional: please add a way to specify if-else behavior

2015-02-12 Thread Louis Wasserman
I get that ifPresent is already available; I'm curious if you examined how
often there is actually an "if absent" case in practice, relative to the
"only do something if present" case.

If you don't have statistics, I could fairly easily get statistics on
Google's codebase for what usages of Guava's Optional look like, in terms
of how often

if (optional.isPresent()) {
   ...
} else {
   ...
}

occurs, relative to

if (optional.isPresent()) {
  ...
}
// no else

On Thu Feb 12 2015 at 10:15:45 AM Paul Sandoz 
wrote:

>
> On Feb 12, 2015, at 7:00 PM, Louis Wasserman  wrote:
>
> > How often does the case when you "have a lambda handy already" come up
> in practice?  If this leads to people using this method instead of
> ifPresent, that seems wasteful.
> >
>
> A lambda bearing ifPresent is already "present" (sorry!) :-) this is just
> filling it out to be consistent for a terminal operation with an empty
> action.
>
>
> > Guava's Optional succeeded as much for what it left out as what it had
> in -- I confess this makes me nervous that j.u.Optional is becoming a
> kitchen sink.
> >
>
> Yes, i most definitely share this concern. I have already closed a few
> optional issues as will not fix. FWIW I don't plan on adding any more stuff.
>
> Paul.
>


Re: Optimization 2.0 for composing strings - Was: Replace concat String to append in StringBuilder parameters

2014-09-08 Thread Louis Wasserman
FWIW, Google has gotten noticeable performance benefits from a change to
javac's compilation of normal + concatenation, to just presize the
StringBuilder.  I haven't had the bandwidth to forward-port that change
yet, unfortunately, but that's a semantics-preserving change that seemed to
us to be within spec.


On Mon, Sep 8, 2014 at 10:50 AM, Jonathan Gibbons <
jonathan.gibb...@oracle.com> wrote:

> Yes, but is this really a big enough performance and footprint pain point
> to be worth addressing in javac itself?
>
> We're now talking about some specific code construction like
> new StringBuilder().append(a + b + c)
>
> Any other case where the string builder can be observed externally cannot
> be subject to the proposed optimization. The use case is now really getting
> pretty specific.
>
> -- Jon
>
>
> On 09/08/2014 10:41 AM, Guy Steele wrote:
>
>> Good point, but counterpoint: it might be acceptable to have modified the
>> string buffer in situations where throwing an exception would always cause
>> the string buffer to become inaccessible.
>>
>> —Guy
>>
>> On Sep 8, 2014, at 1:30 PM, Jonathan Gibbons 
>> wrote:
>>
>>  It would be inappropriate/incorrect to apply the optimization as
>>> described.
>>>
>>> The JLS requires that the argument to a method call should be computed
>>> before invoking the method.
>>>
>>> Consider the case when one of the expressions in the series of string
>>> concatenations throws an exception. It would be incorrect to have already
>>> partially modified the string buffer.
>>>
>>> -- Jon
>>>
>>> On 08/29/2014 01:53 PM, Ulf Zibis wrote:
>>>
>>>> Hi compiler people,
>>>>
>>>> is there some chance that javac could be enhanced to optimize better as
>>>> discussed in this thread? Than refactoring of up to now better readable
>>>> code to ugly StringBuilder@append() code would be superfluous.
>>>> I really like the String concatenation syntax, but unfortunately it
>>>> often causes slower code and bigger footprint.
>>>> Optimally javac would optimize mixed use of StringBuilder,
>>>> StringJoiner, concatenation, toString(), append(String), append(Collection)
>>>> etc. to a single StringBuilder instance, so that the resulting code, JITed
>>>> or not, will have better performance.
>>>> Additionally javac could guess a reasonable initial capacity from the
>>>> given source code.
>>>>
>>>>
>>>> Am 29.08.2014 um 10:01 schrieb Wang Weijun:
>>>>
>>>>> So it's not that the optimization fails but there is no optimization
>>>>> on them yet.
>>>>>
>>>>> I do see the .append("x") case will be easy to deal with, but it looks
>>>>> like historically javac has not been a place to do many optimizations. It
>>>>> mostly converts the java source to byte codes in a 1-to-1 mapping and let
>>>>> VM do whatever it wants (to optimize). When you talked about compiling
>>>>> multiple concatenation into using a single StringBuilder, it's more like
>>>>> choosing the correct implementation rather than an optimization.
>>>>>
>>>>> I don't expect to see big change on this in the near future, so shall
>>>>> we go on with the current enhancement?
>>>>>
>>>>> Thanks
>>>>> Max
>>>>>
>>>>> On Aug 29, 2014, at 2:17, Ulf Zibis  wrote:
>>>>>
>>>>>  I mean:
>>>>>> It does not output byte code that only uses a single char array to
>>>>>> compose the entire String in question.
>>>>>> With "optimization fails", I also mean, there is used an additional
>>>>>> "StringComposer" e.g. another StringBuilder or a StringJoiner in addition
>>>>>> to the 1st StringBuilder.
>>>>>>
>>>>>> -Ulf
>>>>>>
>>>>>> Am 27.08.2014 um 14:02 schrieb Pavel Rappo:
>>>>>>
>>>>>>> Could you please explain what you mean by "javac optimization fails"
>>>>>>> here?
>>>>>>>
>>>>>>> -Pavel
>>>>>>>
>>>>>>> On 27 Aug 2014, at 10:41, Ulf Zibis  wrote:
>>>>>>>
>>>>>>>  4.) Now we see, that javac optimization fails again if
>>>>>>>> StringBuilder, concatenation, toString(), append(String),
>>>>>>>> append(Collection) etc. and StringJoiner use is mixed.
>>>>>>>>
>>>>>>>
>


-- 
Louis Wasserman


Re: FYC: 7197183 : Provide CharSequence.subSequenceView which allows for sub-sequence views of character sequences.

2014-07-16 Thread Louis Wasserman
ng() {
> +int len = length();
> +char[] chars = new char[len];
> +for(int each = 0; each < len; each++) {
> +chars[each] = charAt(each);
> +}
> +return new String(chars, true);
> +}
> +}
> +
> +/**
> + * Returns as a character sequence the specified sub-sequence view of
> the
> + * provided source character sequence. Changes in the source will be
> + * reflected in the sub-sequence. The sub-sequence must, at all
> times, be
> + * a proper sub-sequence of the source character sequence.
> + *
> + * @param source The character sequence from which the sub-sequence is
> + * derived.
> + * @param startInclusive The index of the character in the source
> character
> + * sequence which will be the first character in the sub-sequence.
> + * @param endExclusive The index after the last the character in the
> source
> + * character sequence which will be the last character in the
> sub-sequence
> + * @return the character sub-sequence.
> + * @since 1.9
> + */
> +static CharSequence subSequenceView(CharSequence source, int
> startInclusive, int endExclusive) {
> +return new CharSubSequenceView(source, startInclusive,
> endExclusive);
> +}
> +
> +/**
> + * Returns as a character sequence the specified sub-sequence view of
> the
> + * provided source character sequence. Changes in the source will be
> + * reflected in the sub-sequence. The sub-sequence must, at all
> times, be
> + * a proper sub-sequence of the source character sequence. This
> variation
> + * allows for the size of the sub-sequence to vary, usually to follow
> the
> + * size of a growing character sequence.
> + *
> + * @apiNote The most common usage of this subSequence is to follow
> changes
> + * in the size of the source.
> + * {@code
> + * StringBuilder source = new StringBuilder("prefix:");
> + * CharSeqence toEnd = CharSequence.subSequence(source, 7,
> source::length);
> + * }
> + * In this example the value of {@code toEnd} will always be a
> sub-sequence
> + * of {@code source} but will omit the first 7 characters.
> + *
> + * @param source The character sequence from which the sub-sequence is
> + * derived.
> + * @param startInclusive The index of the character in the source
> character
> + * sequence which will be the first character in the sub-sequence.
> + * @param endExclusive A supplier which returns the index after the
> last the
> + * character in the source character sequence which will be the last
> + * character in the sub-sequence
> + * @return the character sub-sequence.
> + * @since 1.9
> + */
> +static CharSequence subSequenceView(CharSequence source, int
> startInclusive, IntSupplier endExclusive) {
> +return new CharSubSequenceView(source, startInclusive,
> endExclusive);
> +}
>  }
>
>


-- 
Louis Wasserman


Re: DecimalFormat rounding changes in 8 (JEP 177)?

2014-05-04 Thread Louis Wasserman
What does new BigDecimal(1.035).toString() print?  I suspect your issue is
related to the fact that 1.035 is not, in fact, the value represented as a
double; new DecimalFormat("0.00").format(1.035) formats the closest value
representable as a double to the exact value 1.035.

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis


On Sun, May 4, 2014 at 9:56 AM,  wrote:

> Hey,
>
> Could someone please help me understand what changes have happened in
> rounding in DecimalFormat in Java 8?
>
> new DecimalFormat("0.00").format(1.035) is 1.04 on Java 7, and 1.03 on
> Java 8.  (7u25-2.3.10-1~deb7u1, openjdk build 1.8.0_05-b13 debian and
> Oracle build 1.8.0-b132 win64 tested).
>
> My understanding of the RoundingMode.HALF_EVEN (the default)
> documentation is that 1.04 is the correct answer.  In fact,
> (0.000, 1.0035) is 1.004, and (0.0, 1.35) is 1.4.  I am aware
> that floating point is more complex than this, and I am by no
> means an expert.
>
>
> It seems that this may be new code, with a known breaking change in it:
>
> http://openjdk.java.net/jeps/177:
> > Compatibility: On JDK 7, (correct) but altered numerical behavior will
> > only be enabled under an aggressive optimization flag to limit
> > behavioral compatibility impact in that release train. In Java SE 8,
> > the correct numerical behavior will always be required by the
> > specification.
>
> Did this materialise somewhere, and, if so, where's it documented?
>
>
> In summary: My tests fail on Java 8 and I'm not sure they're wrong.
> Any help would be appreciated, thanks.
>
>


Re: ArrayList.removeAll()/retainAll()

2014-02-04 Thread Louis Wasserman
I don't follow.  It looks like ArrayList is throwing an exception on
removeAll(null) -- perfectly valid, in the spec -- not throwing an
exception when the collection contains null.

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis


On Tue, Feb 4, 2014 at 9:00 AM, Benedict Elliott Smith <
belliottsm...@datastax.com> wrote:

> Hi,
>
> I notice this (or a related issue) has been mentioned
> before<
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017663.html
> >on
> this list, but I'm not convinced the correct resolution was reached.
> We
> are seeing this problem thrown by antlr, but rather than a bug in antlr, as
> surmised on the previous exchange, it looks to me that ArrayList is
> imposing a new constraint that is neither declared by itself nor
> Collection, and is unnecessary. ArrayList happily supports null elements,
> so requiring that the provided collection has no null elements is surely a
> bug?
>
> I've pasted the two declarations below for ease of reference. Neither
> javadocs describe the constraint that is imposed.
>
> ArrayList:
> /**
>  * Removes from this list all of its elements that are contained in the
>  * specified collection.
>  *
>  * @param c collection containing elements to be removed from this list
>  * @return {@code true} if this list changed as a result of the call
>  * @throws ClassCastException if the class of an element of this list
>  * is incompatible with the specified collection
>  * (optional)
>  * @throws NullPointerException if this list contains a null element
> and the
>  * specified collection does not permit null elements
>  * (optional),
>  * or if the specified collection is null
>  * @see Collection#contains(Object)
>  */
> public boolean removeAll(Collection c) {
> Objects.requireNonNull(c);
> return batchRemove(c, false);
> }
>
> Collection:
> /**
>  * Removes all of this collection's elements that are also contained in
> the
>  * specified collection (optional operation).  After this call returns,
>  * this collection will contain no elements in common with the
> specified
>  * collection.
>  *
>  * @param c collection containing elements to be removed from this
> collection
>  * @return true if this collection changed as a result of the
>  * call
>  * @throws UnsupportedOperationException if the removeAll
> method
>  * is not supported by this collection
>  * @throws ClassCastException if the types of one or more elements
>  * in this collection are incompatible with the specified
>  * collection
>  * (optional)
>  * @throws NullPointerException if this collection contains one or more
>  * null elements and the specified collection does not support
>  * null elements
>  * (optional),
>  * or if the specified collection is null
>  * @see #remove(Object)
>  * @see #contains(Object)
>  */
> boolean removeAll(Collection c);
>


Re: RFR [8011215] optimization of CopyOnWriteArrayList.addIfAbsent()

2013-04-02 Thread Louis Wasserman
On Tue, Apr 2, 2013 at 3:36 PM, Louis Wasserman  wrote:

> I would be deeply suspicious of benchmarks that naive, especially for
> benchmarks like this that involve lots of allocation -- you're most likely
> benchmarking the GC, not the actual operation.
>

Sorry, let me clarify: can you test how much GC is happening here, and what
proportion of the benchmark time is being taken by GC either way?

The difficulty with benchmarking things like this is that the GC behavior
of a pure benchmark that does lots of allocation tends to be nothing at all
like the GC behavior of that same operation in the middle of a real
application doing lots of other things.  I'd be perfectly comfortable with
this change justified on the grounds of avoiding unnecessary allocation,
but it can be dangerous to make generalizations like "this is X% faster"
when a substantial fraction of one run or the other is just the GC running.


Re: RFR [8011215] optimization of CopyOnWriteArrayList.addIfAbsent()

2013-04-02 Thread Louis Wasserman
I would be deeply suspicious of benchmarks that naive, especially for
benchmarks like this that involve lots of allocation -- you're most likely
benchmarking the GC, not the actual operation.


On Tue, Apr 2, 2013 at 3:33 PM, Ivan Gerasimov wrote:

>
> On 03.04.2013 1:17, Martin Buchholz wrote:
>
>> Have you benchmarked the case where the element is never present?
>>
> That's the only case I've tested.
> If the element were in the array, my code would obviously win.
>
>
>  (with the usual caveats about micro-benchmarking - perhaps use google
>> caliper?)
>>
> The tests I wrote are quite simple - they just run a code snippet for
> several hundreds of times.
> I've just sent and archive with the tests in reply to the other message in
> the thread.
>
>
>
>> On Tue, Apr 2, 2013 at 2:11 PM, Ivan Gerasimov 
>> > ivan.gerasimov@oracle.**com >> wrote:
>>
>>
>> I've done a little testing on my side.
>> I used Integer as an underlying type and set length of the array
>> to the values from 1 to 100.
>> My code shows a little performance gain - approximately 9%.
>> I understand it may not be there for all cases, but at least for
>> some cases it is there.
>>
>>
>


-- 
Louis Wasserman


Re: [concurrency-interest] RFR [8011215] optimization of CopyOnWriteArrayList.addIfAbsent()

2013-04-02 Thread Louis Wasserman
Can we see the implementation of your benchmark?  Accurate benchmarking is
extremely nontrivial.


On Tue, Apr 2, 2013 at 2:34 PM, Ivan Gerasimov wrote:

> Thank you Stanimir!
>
> My main goal was to get rid of early and possibly unneeded memory
> allocation.
> I thought that System.arraycopy() would somehow compensate the need to
> traverse the array twice.
> However testing shows that my code works a bit faster at least when
> dealing with Integer arrays of size from 1 to 100.
>
> Sincerely,
> Ivan
>
> On 02.04.2013 23:25, Stanimir Simeonoff wrote:
>
>> The current version is cache oblivious. In any case for smaller arrays
>> (like COW) System.arrayCopy won't yield any noticeable difference.
>> Also, iirc System.arrayCopy places a memory barrier which in the COW case
>> is unneeded.
>>
>> Stanimir
>>
>>
>>
>> On Tue, Apr 2, 2013 at 9:53 PM, Ivan Gerasimov 
>> > ivan.gerasimov@oracle.**com >> wrote:
>>
>> Hello everybody!
>>
>> Please review my proposal for the
>> CopyOnWriteArrayList.**addIfAbsent() method optimization.
>>
>> http://washi.ru.oracle.com/~**igerasim/webrevs/8011215/**
>> webrev/index.html<http://washi.ru.oracle.com/~igerasim/webrevs/8011215/webrev/index.html>
>> <http://washi.ru.oracle.com/%**7Eigerasim/webrevs/8011215/**
>> webrev/index.html<http://washi.ru.oracle.com/%7Eigerasim/webrevs/8011215/webrev/index.html>
>> >
>>
>>
>> Here is the original function body:
>> --**--
>> Object[] elements = getArray();
>> int len = elements.length;
>> Object[] newElements = new Object[len + 1]; <-- allocate new
>> array in advance
>> for (int i = 0; i < len; ++i) {
>> if (eq(e, elements[i])) <-- check whether
>> e is null on every iteration
>> return false; // exit, throwing away copy
>> else
>> newElements[i] = elements[i];   <-- copy elements
>> one by one
>> }
>> newElements[len] = e;
>> setArray(newElements);
>> --**--
>> The proposed change is to reuse CopyOnWriteArrayList.indexOf()
>> function to check if e is already in the array.
>> If the check passed, new array is allocated withArrays.copyOf().
>> It uses native System.arraycopy(), which is probably faster than
>> copying elements in the loop.
>>
>> Sincerely yours,
>> Ivan
>>
>> __**_
>> Concurrency-interest mailing list
>> Concurrency-interest@cs.**oswego.edu
>> 
>> <mailto:Concurrency-interest@**cs.oswego.edu
>> >
>> 
>> http://cs.oswego.edu/mailman/**listinfo/concurrency-interest<http://cs.oswego.edu/mailman/listinfo/concurrency-interest>
>>
>>
>>
>


-- 
Louis Wasserman


Re: java encoding charset suggestion

2013-03-18 Thread Louis Wasserman
This would probably be likely to break lots of existing users depending on
the default charset, depressingly enough.


On Mon, Mar 18, 2013 at 11:34 AM, Helio Frota  wrote:

> Hi Martin Buchholz,
>
> I believe (but I could be wrong) that only java applications exhibit this
> behavior, programs made with GTK or QT are not affected,
> they probably look at the level of the X11 case are not in variable
> LANG or assume
> a default locale.
>
>
> >  $ (unset LC_ALL LC_COLLATE LANG LANGUAGE GDM_LANG; locale)
> > LANG=
> > LANGUAGE=
> > LC_CTYPE="POSIX"
> > LC_NUMERIC="POSIX"
> > LC_TIME="POSIX"
> > LC_COLLATE="POSIX"
> > LC_MONETARY="POSIX"
> > LC_MESSAGES="POSIX"
> > LC_PAPER="POSIX"
> > LC_NAME="POSIX"
> > LC_ADDRESS="POSIX"
> > LC_TELEPHONE="POSIX"
> > LC_MEASUREMENT="POSIX"
> > LC_IDENTIFICATION="POSIX"
> > LC_ALL=
> >
>
> Please try to use á é í ...
>
> But I think the operating system should set the default, not the
> > application.  On my Ubuntu system I see the traditional ASCII English
> > default:
> >
>
> I agree, but the JVM could not be pro-active ?
>
> Thanks for reply !
>
> 2013/3/18 Martin Buchholz 
>
> > It *would* be nice if the world agreed on using UTF-8 as a universal
> > encoding for all text.  However:
> >
> > Standard says
> > http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html
> >
> > """If the LANG environment variable is not set or is set to the empty
> > string, the implementation-defined default locale shall be used."""
> >
> > But I think the operating system should set the default, not the
> > application.  On my Ubuntu system I see the traditional ASCII English
> > default:
> >
> >  $ (unset LC_ALL LC_COLLATE LANG LANGUAGE GDM_LANG; locale)
> > LANG=
> > LANGUAGE=
> > LC_CTYPE="POSIX"
> > LC_NUMERIC="POSIX"
> > LC_TIME="POSIX"
> > LC_COLLATE="POSIX"
> > LC_MONETARY="POSIX"
> > LC_MESSAGES="POSIX"
> > LC_PAPER="POSIX"
> > LC_NAME="POSIX"
> > LC_ADDRESS="POSIX"
> > LC_TELEPHONE="POSIX"
> > LC_MEASUREMENT="POSIX"
> > LC_IDENTIFICATION="POSIX"
> > LC_ALL=
> >
> >
> > On Mon, Mar 18, 2013 at 11:09 AM, Helio Frota  >wrote:
> >
> >>
> >> I would suggest taking en_US.UTF-8 as default when the LANG variable is
> >> not
> >> set to avoid problems with encoding.
> >>
> >
>
>
> --
> Helio Frota
> http://www.heliofrota.com/
>



-- 
Louis Wasserman


Re: RFR : JDK-8001642 : Add Optional, OptionalDouble, OptionalInt, OptionalLong

2013-03-08 Thread Louis Wasserman
IIRC, there have already been centithreads discussing Optional; I don't
think the final decision is likely to change at this point.


On Fri, Mar 8, 2013 at 12:26 PM, Mike Duigou  wrote:

>
> On Mar 5 2013, at 07:12 , Alan Bateman wrote:
>
> > Just a couple of small things.
> >
> > Will the docs build complain about the unknown taglets? (@apiNote,
> @implNote ...)
>
> It currently issues a warning. I have a Javadoc.gmk patch which I am
> pursuing separately to enable recognition of these tags.
>
> > Are you planning to include a test? I realize this will get a good
> workout once the streams work come in.
>
> Now added.
>
> Thanks!




-- 
Louis Wasserman


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-25 Thread Louis Wasserman
In general, the only thing I did was to mirror the double parsing logic as
closely as I could manage.  If that logic is going to change, then so too
should the patch.  There's really no reason to preserve the specific code I
added in that patch.


On Mon, Feb 25, 2013 at 12:50 PM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> Hi Dima,
>
> On Feb 25, 2013, at 1:14 AM, Dmitry Nadezhin wrote:
>
> I looked at your fix of sunbug=6358355  "Fix Float.parseFloat to round
> correctly and preserve monotonicity".
> […]
>
> 1) The line in floatValue() method
> fValue = (float) doubleValue();
>
> fValue can become Float.POSITIVE_INFINITY.
> It causes incorrect result on input like this
> […]
>
> It can be fixed in such a way
> fValue = Math.max(Float.MIN_VALUE, Math.min(Float.MAX_VALUE, (float)
> doubleValue()));
>
>
> Thanks for the catch!
>
> 2)  The line in floatValue() method.
> // difference is exactly half an ULP
> // round to some other value maybe, then finish
> fValue += 0.5f*ulp( fValue, overvalue );
>
> When fValue is subnormal, the right-hand sign evaluates to 0, fValue
> remains unchanged.
> This is similar to the bug 4396272 - Parsing doubles fails to follow
> IEEE for largest decimal that should yield 0.
> […]
>
>
> It can be fixed in such a way
> fValue = (float) (fValue + 0.5*ulp(fValue, overvalue));
> Constant 0.5 instead of 0.5f causes double evaluation of right-hand
> side, so it doesn'nt underflow.
>
>
> I was intending to comment on this as well.
>
> Once approved, I was thinking that the patch for 7192954 "Fix
> Float.parseFloat to round correctly and preserve monotonicity" should be
> applied first followed by patches for 4396272 "Parsing doubles fails to
> follow IEEE for largest decimal that should yield 0"
> and "BigInteger.doubleValue() is depressingly slow." I initiated some of
> these reviews out of this sequence as I had not become aware of all the
> linkages.
>
> I am actually also wondering whether the patch for 7032154 "Performance
> tuning of sun.misc.FloatingDecimal/FormattedFloatingDecimal," which I just
> learned of today, should perhaps be dealt with prior to any of the above?
>
> Thanks,
>
> Brian
>



-- 
Louis Wasserman


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-22 Thread Louis Wasserman
It's up to you guys, probably.  The issue is that the "reference
implementation" being tested against is wrong, and which order you want to
fix things is up to you guys.


On Fri, Feb 22, 2013 at 11:48 AM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

>
> On Feb 22, 2013, at 11:33 AM, Brian Burkhalter wrote:
>
> Sweet.  Just be aware that the floatValue() optimization requires
> Float.parseFloat to be fixed to match IEEE 754 behavior, as you're
> apparently doing with Double.parseDouble, to pass tests.
>
>
> Actually I missed that until yesterday but found it out myself. I do not
> think however that the patch for 7131192 necessarily has to precede the fix
> for 7192954, as long as the latter gets into the same release.
>
>
> Oops, I had that reversed: I don't know that the patch for 7192954 has to
> precede that for 7131192 if the latter is deemed correct.
>
> B.
>



-- 
Louis Wasserman


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-22 Thread Louis Wasserman
Sweet.  Just be aware that the floatValue() optimization requires
Float.parseFloat to be fixed to match IEEE 754 behavior, as you're
apparently doing with Double.parseDouble, to pass tests.

(The separate patch I sent in to fix that is probably invalidated by your
changes, but it shouldn't be too difficult to do yourself -- it really is
just copy/pasting the Double.parseDouble implementation and using updated
constants.)


On Thu, Feb 21, 2013 at 1:27 PM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> My initial testing with another micro-benchmarking framework (other than
> Caliper) shows a performance increase of >900X for doubleValue() and >1500X
> for floatValue() on a MacBookPro5,3.
>
> There is some more evaluation yet to be done but we should be able to move
> this patch along pretty soon.
>
> Brian
>
> On Feb 15, 2013, at 4:41 PM, Brian Burkhalter wrote:
>
> > Hi Louis,
> >
> > After the two issues for which I've posted review requests (6480539 -
> stripTrailingZeros(), 4396272 - Parsing doubles fails to follow IEEE) are
> resolved, issue 7131192 is currently very near the head of my queue. Given
> that I still have a ways to go to get up to speed on this entire topic area
> and code base, I would hesitate to give a date estimate just yet.
> >
> > Brian
> >
> >> Any update on when this could get reviewed?
> >>
> >>
> >> On Thu, Dec 13, 2012 at 3:36 PM, Louis Wasserman 
> wrote:
> >> Hi,
> >>
> >> I'm working at Google now, but I'd like to revive this patch as a
> contribution from Google.  At the moment, what's mainly needed is review
> for http://bugs.sun.com/view_bug.do?bug_id=7192954, the fix to
> Float.parseFloat's rounding behavior, before we can go anywhere with the
> patch to optimize BigInteger.floatValue() and doubleValue().
> >> Would anyone be able to review that patch so these can start moving
> forward?
> >>
> >> Thanks,
> >> Louis Wasserman
> >> Java Core Libraries Team @ Google
> >> guava-libraries.googlecode.com
>
>


-- 
Louis Wasserman


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2013-02-14 Thread Louis Wasserman
Any update on when this could get reviewed?


On Thu, Dec 13, 2012 at 3:36 PM, Louis Wasserman wrote:

> Hi,
>
> I'm working at Google now, but I'd like to revive this patch as a
> contribution from Google.  At the moment, what's mainly needed is review
> for http://bugs.sun.com/view_bug.do?bug_id=7192954, the fix to
> Float.parseFloat's rounding behavior, before we can go anywhere with the
> patch to optimize BigInteger.floatValue() and doubleValue().
>
>> Would anyone be able to review that patch so these can start moving
> forward?
>
> Thanks,
>
>> Louis Wasserman
> Java Core Libraries Team @ Google
> guava-libraries.googlecode.com
>
> -- Forwarded message --
>> From: Louis Wasserman 
>> Date: Sat, Jul 14, 2012 at 5:32 AM
>> Subject: Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(),
>> floatValue()
>> To: Joseph Darcy 
>> Cc: Andrew Haley , core-libs-dev@openjdk.java.net
>>
>>
>> Understood.  FYI, testing for this change revealed a bug in
>> Float.parseFloat, a patch for which has been separately sent to this
>> mailing list under the subject line "[PATCH] Sunbug 6358355: Rounding error
>> in Float.parseFloat".  (As a result, the BigInteger patch may fail some of
>> the provided tests at the moment, but that is truly because the reference
>> implementation it's being tested against is faulty.)
>>
>> Louis Wasserman
>> wasserman.lo...@gmail.com
>> http://profiles.google.com/wasserman.louis
>>
>>
>> On Sat, Jul 14, 2012 at 2:20 AM, Joseph Darcy wrote:
>>
>>> Hello,
>>>
>>> Thanks for the patch Louis.
>>>
>>>
>>> On 7/12/2012 3:21 AM, Andrew Haley wrote:
>>>
>>>> On 07/12/2012 10:32 AM, Louis Wasserman wrote:
>>>>
>>>>> It was attached to the previous message?  I don't know if this list
>>>>> works
>>>>> with attachments.  Alternately, the patch was attached here:
>>>>> https://bugs.openjdk.java.net/**show_bug.cgi?id=100222<https://bugs.openjdk.java.net/show_bug.cgi?id=100222>
>>>>>
>>>>> I'm not sure what you mean by double-rounding bugs, though.  It's
>>>>> not difficult to actually implement the HALF_EVEN rounding behavior
>>>>> with bit twiddling.
>>>>>
>>>> Sure, as long as you've thought about it and done it carefully.  The
>>>> bit twiddling is easy to do, and easy to get wrong.
>>>>
>>>> > From the supplied patch it looks like you've done a good job, but
>>>> there was no way to tell without it.  I presume the listserv dropped
>>>> it on the floor.
>>>>
>>>> Andrew.
>>>>
>>>
>>> I've taken a quick look at the patch.  The concept for the change is
>>> good; the current path of converting to float/double through a string is a
>>> simple but very roundabout way to accomplish this task.
>>>
>>> Unfortunately, I'm saturated with the JDK bug migration [1] and will
>>> continue to be saturated for at least several more weeks so I won't be able
>>> to take a more detailed look at the patch for a while.  I suspect some more
>>> directly test cases will be needed to test tricky rounding situations.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>> [1] 
>>> https://blogs.oracle.com/**darcy/entry/moving_monarchs_**dragons<https://blogs.oracle.com/darcy/entry/moving_monarchs_dragons>
>>>
>>
>>
>>
>
>
> --
> Louis Wasserman
>



-- 
Louis Wasserman


Re: JDK 8 code review request for 6964528: Double.toHexString(double d) String manipulation with + in an append of StringBuilder

2013-02-01 Thread Louis Wasserman
Would appending the character 'p' instead of the string "p" make any
difference?


On Fri, Feb 1, 2013 at 1:39 PM, Joe Darcy  wrote:

> Hello,
>
> Please review a simple refactoring fix in Double.toHexString
>
> 6964528: Double.toHexString(double d) String manipulation with + in an
> append of StringBuilder
> 
> http://cr.openjdk.java.net/~**darcy/6964528.0/<http://cr.openjdk.java.net/~darcy/6964528.0/>
>
> Patch below.
>
> Thanks,
>
> -Joe
>
> --- old/src/share/classes/java/**lang/Double.java2013-02-01
> 13:36:33.0 -0800
> +++ new/src/share/classes/java/**lang/Double.java2013-02-01
> 13:36:33.0 -0800
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 1994, 2012, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 1994, 2013, 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
> @@ -289,7 +289,7 @@
>  return Double.toString(d);
>  else {
>  // Initialized to maximum size of output.
> -StringBuffer answer = new StringBuffer(24);
> +StringBuilder answer = new StringBuilder(24);
>
>  if (Math.copySign(1.0, d) == -1.0)// value is negative,
>  answer.append("-");  // so append sign
> info
> @@ -300,8 +300,7 @@
>
>  if(d == 0.0) {
>  answer.append("0.0p0");
> -}
> -else {
> +} else {
>  boolean subnormal = (d < DoubleConsts.MIN_NORMAL);
>
>  // Isolate significand bits and OR in a high-order bit
> @@ -324,13 +323,14 @@
>"0":
>signif.replaceFirst("0{1,12}$"**, ""));
>
> +answer.append("p");
>  // If the value is subnormal, use the E_min exponent
>  // value for double; otherwise, extract and report d's
>  // exponent (the representation of a subnormal uses
>  // E_min -1).
> -answer.append("p" + (subnormal ?
> -   DoubleConsts.MIN_EXPONENT:
> -   Math.getExponent(d) ));
> +answer.append(subnormal ?
> +  DoubleConsts.MIN_EXPONENT:
> +  Math.getExponent(d));
>  }
>  return answer.toString();
>  }
>
>


-- 
Louis Wasserman


Re: Codereview request for 8006295: Base64.Decoder.wrap(java.io.InputStream) returns InputStream which throws unspecified IOException on attempt to decode invalid Base64 byte stream

2013-01-31 Thread Louis Wasserman
In what way is this "obviously" inappropriate?  (I don't object, I just
don't follow.)


On Thu, Jan 31, 2013 at 2:31 PM, Lance Andersen - Oracle <
lance.ander...@oracle.com> wrote:

> +1
> On Jan 31, 2013, at 5:15 PM, Xueming Shen wrote:
>
> > Hi,
> >
> > Obviously ioe is not an appropriate exception for invalid base64 bytes,
> > and it's inconsistent with the rest of decode methods, as the submitter
> > suggested.
> > The change is to explicitly specify (as other decoding methods do) that
> > IAE will be thrown if the input stream contains invalid base64 bytes. The
> > impl is also updated according.
> >
> > Please help review.
> >
> > http://cr.openjdk.java.net/~sherman/8006295/webrev/
> >
> > Thanks
> > -Sherman
>
>
>
> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> lance.ander...@oracle.com
>
>
>


-- 
Louis Wasserman


Re: RFR 8005311: Add Scalable Updatable Variables, DoubleAccumulator, DoubleAdder, LongAccumulator, LongAdder

2013-01-07 Thread Louis Wasserman
Hmmm.  Is that algorithm doable concurrently without needing a 128-bit CAS?


On Mon, Jan 7, 2013 at 11:07 AM, Joe Darcy  wrote:

> Hello,
>
> I had a question about how the double accumulation logic was intended to
> be used.  I've taken a quick look at the code and it uses straightforward
> "sum = sum + nextValue" code to compute the double sum.  Summing doubles
> values with code numerical accuracy is surprisingly tricky and if the
> DoubleAccumulator code is meant for wide use, I'd recommend using instead
> some form of compensated summation:
>
> 
> http://en.wikipedia.org/wiki/**Kahan_summation_algorithm<http://en.wikipedia.org/wiki/Kahan_summation_algorithm>
>
> Thanks,
>
> -Joe
>
>
> On 1/5/2013 10:10 AM, Chris Hegarty wrote:
>
>> As part of JEP 155 we are proposing to add the following public classes
>> to support Scalable Updatable Variables, DoubleAccumulator, DoubleAdder,
>> LongAccumulator and LongAdder.
>>
>> These have been written by Doug Lea, with assistance from members of the
>> former JCP JSR-166 Expert Group.
>>
>> Webrev and javadoc are at:
>>   
>> http://cr.openjdk.java.net/~**chegar/8005311/ver.00/<http://cr.openjdk.java.net/~chegar/8005311/ver.00/>
>>
>> Since Doug is the author, I am taking a reviewer/sponsor role.
>>
>> Here are my initial comments.
>>  - There are various places in DoubleAccmulator where there are broken
>>links to #sum ( I think it is just a cut'n'paste error ). These
>>should be #get.
>>  - Accumulators
>>{@link #get} may read somewhat better as {@link #get current value} ??
>>  - Accumulators
>>Does the 'identity' value need further explanation?
>>
>> Note: There is one minor change to the implementation. Currently in the
>> jdk8 repo j.u.f.DoubleBinaryOperator defines operateAsDouble. This method
>> has been renamed to applyAsDouble in the lambda/lambda repo. When these
>> changes are sync'ed from lambda/lambda this can be reverted. A similar
>> comment has been added to the code.
>>
>> -Chris.
>>
>
>


-- 
Louis Wasserman


Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Louis Wasserman
Could we say that, in so many words, in the Javadoc?


On Tue, Dec 18, 2012 at 4:12 PM, David Holmes wrote:

> On 19/12/2012 8:40 AM, Louis Wasserman wrote:
>
>> It's not 100% obvious to me whether this refers to a default
>> implementation
>> in an interface, a class which inherits that default implementation and
>> does not override it, or both.  Is that worth clarifying in the doc,
>> rather
>> than forcing readers to check the JLS citation?
>>
>
> The issue is where you obtained this Method reference from:
>
> - from the Interface? then it is a default method
> - from a class implementing the interface but not redefining the method?
> then it is a default method
> - from a class implementing the interface and redefining the method? then
> it is NOT a default method.
>
> David
> -
>
>
>>
>> On Tue, Dec 18, 2012 at 2:32 PM, Joe Darcy  wrote:
>>
>>  Mandy and Jim,
>>>
>>> I'll correct the typos before I push.
>>>
>>> Thanks for the careful review,
>>>
>>> -Joe
>>>
>>>
>>> On 12/18/2012 01:21 PM, Mandy Chung wrote:
>>>
>>>  On 12/18/12 12:43 PM, Joe Darcy wrote:
>>>>
>>>>  Hello,
>>>>>
>>>>> Please review these changes to add core reflection support to recognize
>>>>> default methods:
>>>>>
>>>>>  8005042 Add Method.isDefault to core reflection
>>>>> http://cr.openjdk.java.net/~darcy/8005042.0/<http://cr.openjdk.java.net/~**darcy/8005042.0/>
>>>>> <http://cr.**openjdk.java.net/~darcy/**8005042.0/<http://cr.openjdk.java.net/~darcy/8005042.0/>
>>>>> >
>>>>>
>>>>>
>>>>>  Looks good to me.
>>>>
>>>> For the test:
>>>> 56   System.err.printf("ERROR: On %s expected isDefualt of ''%s'';
>>>> got
>>>> ''%s''.\n",
>>>> 57 method.toString(), expected, actual);
>>>>
>>>> A typo 'isDefualt' ->  'isDefault'.  This uses two single-quote
>>>> characters
>>>> to wrap the expected and actual value - is it intentional?  I was
>>>> wondering
>>>> that you meant to use one singe-quote character.
>>>>
>>>> Mandy
>>>>
>>>>
>>>>
>>>
>>
>>


-- 
Louis Wasserman


Re: JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

2012-12-18 Thread Louis Wasserman
It's not 100% obvious to me whether this refers to a default implementation
in an interface, a class which inherits that default implementation and
does not override it, or both.  Is that worth clarifying in the doc, rather
than forcing readers to check the JLS citation?


On Tue, Dec 18, 2012 at 2:32 PM, Joe Darcy  wrote:

> Mandy and Jim,
>
> I'll correct the typos before I push.
>
> Thanks for the careful review,
>
> -Joe
>
>
> On 12/18/2012 01:21 PM, Mandy Chung wrote:
>
>> On 12/18/12 12:43 PM, Joe Darcy wrote:
>>
>>> Hello,
>>>
>>> Please review these changes to add core reflection support to recognize
>>> default methods:
>>>
>>> 8005042 Add Method.isDefault to core reflection
>>> http://cr.openjdk.java.net/~**darcy/8005042.0/<http://cr.openjdk.java.net/~darcy/8005042.0/>
>>>
>>>
>> Looks good to me.
>>
>> For the test:
>>56   System.err.printf("ERROR: On %s expected isDefualt of ''%s''; got
>> ''%s''.\n",
>>57 method.toString(), expected, actual);
>>
>> A typo 'isDefualt' -> 'isDefault'.  This uses two single-quote characters
>> to wrap the expected and actual value - is it intentional?  I was wondering
>> that you meant to use one singe-quote character.
>>
>> Mandy
>>
>>
>


-- 
Louis Wasserman


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2012-12-13 Thread Louis Wasserman
Hi,

I'm working at Google now, but I'd like to revive this patch as a
contribution from Google.  At the moment, what's mainly needed is review
for http://bugs.sun.com/view_bug.do?bug_id=7192954, the fix to
Float.parseFloat's rounding behavior, before we can go anywhere with the
patch to optimize BigInteger.floatValue() and doubleValue().

> Would anyone be able to review that patch so these can start moving
forward?

Thanks,

> Louis Wasserman
Java Core Libraries Team @ Google
guava-libraries.googlecode.com

-- Forwarded message --
> From: Louis Wasserman 
> Date: Sat, Jul 14, 2012 at 5:32 AM
> Subject: Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(),
> floatValue()
> To: Joseph Darcy 
> Cc: Andrew Haley , core-libs-dev@openjdk.java.net
>
>
> Understood.  FYI, testing for this change revealed a bug in
> Float.parseFloat, a patch for which has been separately sent to this
> mailing list under the subject line "[PATCH] Sunbug 6358355: Rounding error
> in Float.parseFloat".  (As a result, the BigInteger patch may fail some of
> the provided tests at the moment, but that is truly because the reference
> implementation it's being tested against is faulty.)
>
> Louis Wasserman
> wasserman.lo...@gmail.com
> http://profiles.google.com/wasserman.louis
>
>
> On Sat, Jul 14, 2012 at 2:20 AM, Joseph Darcy wrote:
>
>> Hello,
>>
>> Thanks for the patch Louis.
>>
>>
>> On 7/12/2012 3:21 AM, Andrew Haley wrote:
>>
>>> On 07/12/2012 10:32 AM, Louis Wasserman wrote:
>>>
>>>> It was attached to the previous message?  I don't know if this list
>>>> works
>>>> with attachments.  Alternately, the patch was attached here:
>>>> https://bugs.openjdk.java.net/**show_bug.cgi?id=100222<https://bugs.openjdk.java.net/show_bug.cgi?id=100222>
>>>>
>>>> I'm not sure what you mean by double-rounding bugs, though.  It's
>>>> not difficult to actually implement the HALF_EVEN rounding behavior
>>>> with bit twiddling.
>>>>
>>> Sure, as long as you've thought about it and done it carefully.  The
>>> bit twiddling is easy to do, and easy to get wrong.
>>>
>>> > From the supplied patch it looks like you've done a good job, but
>>> there was no way to tell without it.  I presume the listserv dropped
>>> it on the floor.
>>>
>>> Andrew.
>>>
>>
>> I've taken a quick look at the patch.  The concept for the change is
>> good; the current path of converting to float/double through a string is a
>> simple but very roundabout way to accomplish this task.
>>
>> Unfortunately, I'm saturated with the JDK bug migration [1] and will
>> continue to be saturated for at least several more weeks so I won't be able
>> to take a more detailed look at the patch for a while.  I suspect some more
>> directly test cases will be needed to test tricky rounding situations.
>>
>> Thanks,
>>
>> -Joe
>>
>> [1] 
>> https://blogs.oracle.com/**darcy/entry/moving_monarchs_**dragons<https://blogs.oracle.com/darcy/entry/moving_monarchs_dragons>
>>
>
>
>


-- 
Louis Wasserman


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2012-07-14 Thread Louis Wasserman
Understood.  FYI, testing for this change revealed a bug in
Float.parseFloat, a patch for which has been separately sent to this
mailing list under the subject line "[PATCH] Sunbug 6358355: Rounding error
in Float.parseFloat".  (As a result, the BigInteger patch may fail some of
the provided tests at the moment, but that is truly because the reference
implementation it's being tested against is faulty.)

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis


On Sat, Jul 14, 2012 at 2:20 AM, Joseph Darcy  wrote:

> Hello,
>
> Thanks for the patch Louis.
>
>
> On 7/12/2012 3:21 AM, Andrew Haley wrote:
>
>> On 07/12/2012 10:32 AM, Louis Wasserman wrote:
>>
>>> It was attached to the previous message?  I don't know if this list works
>>> with attachments.  Alternately, the patch was attached here:
>>> https://bugs.openjdk.java.net/**show_bug.cgi?id=100222<https://bugs.openjdk.java.net/show_bug.cgi?id=100222>
>>>
>>> I'm not sure what you mean by double-rounding bugs, though.  It's
>>> not difficult to actually implement the HALF_EVEN rounding behavior
>>> with bit twiddling.
>>>
>> Sure, as long as you've thought about it and done it carefully.  The
>> bit twiddling is easy to do, and easy to get wrong.
>>
>> > From the supplied patch it looks like you've done a good job, but
>> there was no way to tell without it.  I presume the listserv dropped
>> it on the floor.
>>
>> Andrew.
>>
>
> I've taken a quick look at the patch.  The concept for the change is good;
> the current path of converting to float/double through a string is a simple
> but very roundabout way to accomplish this task.
>
> Unfortunately, I'm saturated with the JDK bug migration [1] and will
> continue to be saturated for at least several more weeks so I won't be able
> to take a more detailed look at the patch for a while.  I suspect some more
> directly test cases will be needed to test tricky rounding situations.
>
> Thanks,
>
> -Joe
>
> [1] 
> https://blogs.oracle.com/**darcy/entry/moving_monarchs_**dragons<https://blogs.oracle.com/darcy/entry/moving_monarchs_dragons>
>


[PATCH] Sunbug 6358355: Rounding error in Float.parseFloat

2012-07-12 Thread Louis Wasserman
Float.parseFloat doesn't currently round correctly, or even monotonically,
in certain cases.

In particular, the following test program prints false:


public class Foo {
  public static void main(String[] args) {
System.out.println(144115196665790480f <= 144115196665790481f);
  }
}

A patch is attached, and can also be found at
https://bugs.openjdk.java.net/show_bug.cgi?id=100208.

There was a comment in sun.misc.FloatingDecimal claiming this would take
400 lines of code, but by eliminating the (fallacious) "sticky rounding"
logic, and just duplicating the double-parsing logic, it only ends up
costing ~40 net lines of code added.

The added code is mostly identical to the preexisting double-parsing code.

This is a prerequisite for the separate, previously sent patch improving
the performance of BigInteger.doubleValue() and floatValue().  (Testing for
that patch revealed this bug.)

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis


Re: [PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2012-07-12 Thread Louis Wasserman
It was attached to the previous message?  I don't know if this list works
with attachments.  Alternately, the patch was attached here:
https://bugs.openjdk.java.net/show_bug.cgi?id=100222

I'm not sure what you mean by double-rounding bugs, though.  It's not
difficult to actually implement the HALF_EVEN rounding behavior with bit
twiddling.  That said, I *did* encounter places where prior erroneous
behavior (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6358355) of
Float.parseFloat caused my correct implementation to fail tests...

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis


On Thu, Jul 12, 2012 at 7:58 AM, Andrew Haley  wrote:

> On 07/11/2012 11:05 PM, Louis Wasserman wrote:
> > Doing it with bit-twiddling and Double.longBitsToDouble improves the
> speed
> > of those methods by something like two orders of magnitude in my
> benchmarks.
>
> Mmm, but that's going to hit double-rounding bugs for large numbers.  Where
> is this patch, anyway?
>
> Andrew.
>


[PATCH] Sunbug 7131192: Optimize BigInteger.doubleValue(), floatValue()

2012-07-11 Thread Louis Wasserman
Not entirely sure I'm posting this in the right place, but it looks like
it...

This is a patch to optimize BigInteger.doubleValue() and floatValue(),
which are both currently implemented by converting to a string and back.

Doing it with bit-twiddling and Double.longBitsToDouble improves the speed
of those methods by something like two orders of magnitude in my benchmarks.

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis


Float.parseFloat rounding patch

2012-02-08 Thread Louis Wasserman
A while back I independently discovered Sun bug
6358355<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6358355> --
basically, under certain conditions, Float.parseFloat just uses a heuristic
to round from Double.parseDouble (it tries to round in the opposite
direction from parseDouble), which predictably enough for a hackish
approach to floating-point rounding, doesn't work correctly in all cases.
 The relevant source is
here<https://bugs.openjdk.java.net/attachment.cgi?id=243&action=diff#a/src/share/classes/sun/misc/FloatingDecimal.java_sec5>;
the comment: "look at the class instance variable roundDir, which should
help us avoid double-rounding error. roundDir was set in hardValueOf if the
estimate was close enough, but not exact. It tells us which direction of
rounding is preferred."

I have a patch and a test at
https://bugs.openjdk.java.net/show_bug.cgi?id=100208, and I was advised to
contact this mailing list to get it reviewed.  What do I need to do?

Louis Wasserman
wasserman.lo...@gmail.com
http://profiles.google.com/wasserman.louis