Some tickets fixed + suggested improvements for SIP-14

[√iktor Ҡlang]

Feedback?

https://github.com/viktorklang/scala/pull/5/files

--

Viktor Klang
Director of Engineering

Typesafe - The software stack for applications that scale
Twitter: @viktorklang

[Daniel Sobral]

I see implementation changes, but I'm not sure what is qualifying as
suggested improvement. I don't think scala-sips is a good place to ask
for review on implementation.

--
Daniel C. Sobral

I travel to the future all the time.

[√iktor Ҡlang]

On Mon, Jan 14, 2013 at 4:22 PM, Daniel Sobral <dcso...@gmail.com> wrote:

I see implementation changes, but I'm not sure what is qualifying as
suggested improvement. I don't think scala-sips is a good place to ask
for review on implementation.

Ok, I posted it here as well due to criticism in the past that work was being done in private.

You are not required to give feedback, but I haven't been candid about the changes at least.

Cheers,

√

 

On 14 January 2013 09:56, √iktor Ҡlang <viktor...@gmail.com> wrote:
> Feedback?
>
> https://github.com/viktorklang/scala/pull/5/files
>
> --
> Viktor Klang
> Director of Engineering
>
> Typesafe - The software stack for applications that scale
> Twitter: @viktorklang

--
Daniel C. Sobral

I travel to the future all the time.

[Mushtaq Ahmed]

I wonder why fallbackTo does not take by-name param, given that it is analogous to getOrElse?

[Mushtaq Ahmed]

On Sat, Jan 26, 2013 at 4:17 PM, Mushtaq Ahmed <mush...@gmail.com> wrote:

I wonder why fallbackTo does not take by-name param, given that it is analogous to getOrElse?

I mean orElse. 

[√iktor Ҡlang]

What exception would you expect to be in the resulting Future of "orElse" if both "this" and "that" fails?

Cheers,

√

[Mushtaq Ahmed]

The exception of "this" (as it is now). Will it not be possible if we change the signature like below? 

def fallbackTo[U >: T](that: => Future[U]): Future[U]

[√iktor Ҡlang]

On Sat, Jan 26, 2013 at 1:15 PM, Mushtaq Ahmed <mush...@gmail.com> wrote:

The exception of "this" (as it is now). Will it not be possible if we change the signature like below? 

def fallbackTo[U >: T](that: => Future[U]): Future[U]

Question is if we should deprecate fallbackTo and introduce orElse with a lazy parameter, thoughts?

Cheers,

√

 

On Sat, Jan 26, 2013 at 4:31 PM, √iktor Ҡlang <viktor...@gmail.com> wrote:

What exception would you expect to be in the resulting Future of "orElse" if both "this" and "that" fails?

Cheers,

√

On Sat, Jan 26, 2013 at 11:55 AM, Mushtaq Ahmed <mush...@gmail.com> wrote:

On Sat, Jan 26, 2013 at 4:17 PM, Mushtaq Ahmed <mush...@gmail.com> wrote:

I wonder why fallbackTo does not take by-name param, given that it is analogous to getOrElse?

I mean orElse. 

--

Viktor Klang
Director of Engineering

Typesafe - The software stack for applications that scale
Twitter: @viktorklang

[Rex Kerr]

I never understood why fallbackTo had the name it had and the signature it had.  I'd prefer an orElse with the standard orElse signature (cf. Option, Try, PartialFunction).

  --Rex

[Pavel Pavlov]

+1

[√iktor Ҡlang]

Because you lose parallelizability if the rhs is lazy. FallbackTo is extremely useful for farming out work and picking the first success.

Of course you can do that with a lazy rhs but it is more work, i.e. starting computations and the assigning to vals, then passing those in. There's also more overhead (thunk allocations).

(Also, all by-name args in concurrency API introduces a vector of closing over things you didn't intend to.)

That said, I'm not against introducing orElse.

Cheers,
V

[√iktor Ҡlang]

As a curious fact, Try.orElse does not retain the original failure:

  /** Returns this `Try` if it's a `Success` or the given `default` argument if this is a `Failure`.

   */

  def orElse[U >: T](default: => Try[U]): Try[U] =

    try if (isSuccess) this else default

    catch {

      case NonFatal(e) => Failure(e)

    }

[√iktor Ҡlang]

Also, as a result of this discovery I think we either need to:

a) change Try.orElse

b) have Future.orElse mirror it in behavior, which leads to retaining fallbackTo since it has different semantics: rhs is strict and first failure is retained.

Thoughts?

Cheers,

√

[√iktor Ҡlang]

Or:

c) Leaving Future.fallback to as it were, but changing the docs to show that it will contain the last failure if everything fails.

[Rex Kerr]

On Sat, Jan 26, 2013 at 4:22 PM, √iktor Ҡlang <viktor...@gmail.com> wrote:

Because you lose parallelizability if the rhs is lazy. FallbackTo is extremely useful for farming out work and picking the first success.

Ah, okay.  I hadn't considered that scenario explicitly.  In that case I vote for orElse as well rather than instead of fallbackTo, and for a very clear description in the Scaladocs explaining why and when one would wish to use each (including the first error vs. last error thing).

  --Rex

 

--
 
 

[√iktor Ҡlang]

On Sat, Jan 26, 2013 at 11:26 PM, Rex Kerr <ich...@gmail.com> wrote:

On Sat, Jan 26, 2013 at 4:22 PM, √iktor Ҡlang <viktor...@gmail.com> wrote:

Because you lose parallelizability if the rhs is lazy. FallbackTo is extremely useful for farming out work and picking the first success.

Ah, okay.  I hadn't considered that scenario explicitly.  In that case I vote for orElse as well rather than instead of fallbackTo, and for a very clear description in the Scaladocs explaining why and when one would wish to use each (including the first error vs. last error thing).

That sounds like a good middle road to me.

Anyone else?

Cheers,

√

[Mushtaq Ahmed]

On Sun, Jan 27, 2013 at 4:21 AM, √iktor Ҡlang <viktor...@gmail.com> wrote:

On Sat, Jan 26, 2013 at 11:26 PM, Rex Kerr <ich...@gmail.com> wrote:

Ah, okay.  I hadn't considered that scenario explicitly.  In that case I vote for orElse as well rather than instead of fallbackTo, and for a very clear description in the Scaladocs explaining why and when one would wish to use each (including the first error vs. last error thing).

That sounds like a good middle road to me.

Anyone else?

Sounds good to me.

 

[√iktor Ҡlang]

So: orElse == recoverWith?        def orElse[U >: T](that: => Future[U]): Future[U] = recoverWith { case _ => that }

or perhaps more efficiently something like:

def orElse[U >: T](that: => Future[U]): Future[U] = {

  val p = Promise[U]()

    onComplete {

      case Failure(t) => try p completeWith that catch { case NonFatal(t) => p failure t }

      case other => p complete other

    }

    p.future

  }

[Mushtaq Ahmed]

So the middle ground is:

orElse: you get short-circuiting, but the error comes from 'that'

fallbackTo: you get parallelism, but the error comes from 'this'

This is a bit subtle to remember. If we could fix the error part to be same in both cases, it may be easier.

--
 
 

[√iktor Ҡlang]

On Sun, Jan 27, 2013 at 2:59 PM, Mushtaq Ahmed <mush...@gmail.com> wrote:

So the middle ground is:

orElse: you get short-circuiting, but the error comes from 'that'

fallbackTo: you get parallelism, but the error comes from 'this'

This is a bit subtle to remember. If we could fix the error part to be same in both cases, it may be easier.

Well, in 2.10.0 fallbackTo has the error of that, but was documented to be the error of this, it was logged as an error, and the proposed fix for 2.10.1/2.11 is to be the error of this. The alternative is to keep fallbackTo as-is and introduce orElse as the short-circuit version.

Thoughts?

Cheers,

√

 

On Sun, Jan 27, 2013 at 3:58 PM, √iktor Ҡlang <viktor...@gmail.com> wrote:

So: orElse == recoverWith?        def orElse[U >: T](that: => Future[U]): Future[U] = recoverWith { case _ => that }

or perhaps more efficiently something like:

def orElse[U >: T](that: => Future[U]): Future[U] = {

  val p = Promise[U]()

    onComplete {

      case Failure(t) => try p completeWith that catch { case NonFatal(t) => p failure t }

      case other => p complete other

    }

    p.future

  }

On Sun, Jan 27, 2013 at 3:40 AM, Mushtaq Ahmed <mush...@gmail.com> wrote:

On Sun, Jan 27, 2013 at 4:21 AM, √iktor Ҡlang <viktor...@gmail.com> wrote:

On Sat, Jan 26, 2013 at 11:26 PM, Rex Kerr <ich...@gmail.com> wrote:

Ah, okay.  I hadn't considered that scenario explicitly.  In that case I vote for orElse as well rather than instead of fallbackTo, and for a very clear description in the Scaladocs explaining why and when one would wish to use each (including the first error vs. last error thing).

That sounds like a good middle road to me.

Anyone else?

Sounds good to me.

 

--

Viktor Klang
Director of Engineering

Typesafe - The software stack for applications that scale
Twitter: @viktorklang

--
 
 

--
 
 

[√iktor Ҡlang]

The problem though, is that they will just be syntactic sugar for recoverWith, and the question is how the added API adds enough value to warrant the larger surface area.

[Mushtaq Ahmed]

On Sun, Jan 27, 2013 at 8:07 PM, √iktor Ҡlang <viktor...@gmail.com> wrote:

The problem though, is that they will just be syntactic sugar for recoverWith, and the question is how the added API adds enough value to warrant the larger surface area.

If I have to vote for one of them, it will be orElse. In many places in our codebase 'then' part fires a reactivemongo query, hence need to be lazy. 

If it comes bundled in the lib, great. Otherwise, we can always write our own using the recoverWith. I have not used fallbackTo at all hence won't mind if it does go out.