AbstractSerializer iterates iterable twice causing huge scala and java impact #107

Closed
deanhiller opened this Issue Aug 22, 2012 · 2 comments

Projects

None yet

2 participants

@deanhiller

Scala is known for passing views of iterables down so you can combine all for loops into one. We do the same in java in playOrm and we pass astyanax and Iterable that proxies about 3 other Iterables each doing transforms. Astyanax ends up invoking these all twice with this code...

public List<ByteBuffer> toBytesList(Iterable<T> list) {
   //It is invoked ONCE here when you call size....
    List<ByteBuffer> bytesList = new ArrayList<ByteBuffer>(Iterables.size(list));
   //It is invoked again here when you for loop
    for (T s : list) {
        bytesList.add(toByteBuffer(s));
    }
    return bytesList;
}

This is kind of a bad practice for those clients that will pass in an Iterable since you will call up the chain twice and cause transforms at every layer to happen twice. Work around is to loop myself right now before I pass it to astyanax preventing the double transform(but I now have two for loops on the keys.....one in astyanax and one in my code that would not have been needed)

thanks,
Dean

@elandau
Member
elandau commented Dec 4, 2012

Code checked in and will be fixed in 1.56.20

@elandau elandau closed this Dec 4, 2012
@deanhiller

sweeet, thanks, Dean

@DavidHerzogTU-Berlin DavidHerzogTU-Berlin pushed a commit to DavidHerzogTU-Berlin/astyanax that referenced this issue Nov 5, 2014
@elandau elandau Fix for Netflix/astyanax#107
Fix distributed lock not being applied due to bad timestamps
c88bab2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment