-3

I have two snippets of code which are technically same, but the second one takes 1 sec extra then the first one. The first one executes in 6 sec and the second in 7.

Double yearlyEarnings = employmentService.getYearlyEarningForUserWithEmployer(userId, emp.getId());

CompletableFuture<Double> earlyEarningsInHomeCountryCF = currencyConvCF.thenApplyAsync(currencyConv -> {
  return currencyConv * yearlyEarnings;
});

The above one takes 6s and the next takes 7s Here is the link to code

CompletableFuture<Double> earlyEarningsInHomeCountryCF = currencyConvCF.thenApplyAsync(currencyConv -> {
       Double yearlyEarnings = employmentService.getYearlyEarningForUserWithEmployer(userId, emp.getId());
       return currencyConv * yearlyEarnings;  
 });

Please explain why the second code consistently takes 1s more (extra time) as compared to the first one

Below is the signature of the method getYearlyEarningForUserWithEmployer. Just sharing, but it should not have any affect

Double getYearlyEarningForUserWithEmployer(long userId, long employerId);

Here is the link to code

Robin
  • 6,879
  • 7
  • 37
  • 35
  • Can you give snippet of employmentService.getYearlyEarningForUserWithEmployer() – Naruto Mar 27 '16 at 20:42
  • How does it make a difference ? – Robin Mar 27 '16 at 20:47
  • Please make a [mcve]. – Tunaki Mar 27 '16 at 20:48
  • Could you post it in the question itself? – Tunaki Mar 27 '16 at 21:13
  • Better to simplify the var names and put the complete example so we can duplicate it from out side – Muhammad Hewedy Mar 28 '16 at 13:13
  • Your question does not only fail to show the relevant code, it also lacks an explanation of *what* takes six or seven seconds. Your second code fragment consists of a sole `thenApplyAsync` invocation which will return *immediately*, no matter what the lambda expression contains, so it executes on a nanosecond scale. You’re obviously measuring an entirely different operation, most likely containing at least one wait-for-completion operation. You can’t expect answers regarding that issue without posting what you are actually doing. – Holger Mar 29 '16 at 17:38

2 Answers2

0

Your question is horribly incomplete, but from what we can guess, it’s entirely plausible that the second variant takes longer, if we assume that currencyConvCF represents an asynchronous operation which might be running concurrently while your code fragments are executed and you’re talking about the overall time it takes to complete all operations, including the one represented by the CompletableFuture returned by thenApplyAsync (earlyEarningsInHomeCountryCF).

In the first variant you are invoking getYearlyEarningForUserWithEmployer while the operation represented by currencyConvCF might be still running concurrently. The multiplication will happen when both operations completed.

In the second variant, the getYearlyEarningForUserWithEmployer invocation is part of the operation passed to currencyConvCF.thenApplyAsync, thus it will not start before the operation represented by currencyConvCF has been completed, so no operation will run concurrently. If we assume that getYearlyEarningForUserWithEmployer takes a significant time, say one second, and has no internal dependencies to the other operation, it’s not surprising when the overall operation takes longer in that variant.

It seems, what you actually want to do is something like:

CompletableFuture<Double> earlyEarningsInHomeCountryCF = currencyConvCF.thenCombineAsync(
    CompletableFuture.supplyAsync(
        () -> employmentService.getYearlyEarningForUserWithEmployer(userId, emp.getId())),
    (currencyConv, yearlyEarnings) -> currencyConv * yearlyEarnings);

so getYearlyEarningForUserWithEmployer is not executed sequentially in the initiating thread but both source operations can run asynchronously before the final multiplication applies.

However, when you are invoking get right afterwards in the initiating thread, like in your linked code on github, that asynchronous processing of the second operation has no benefit. Instead of waiting for the completion, your initiating thread can just perform the independent operation as the second code variant of your question already does and you will likely be even faster when not spawning an asynchronous operation for something as simple as a single multiplication, i.e. use instead:

CompletableFuture<Double> currencyConvCF = /* a true asynchronous operation */
return employmentService.getYearlyEarningForUserWithEmployer(userId, emp.getId())
     * employerCurrencyCF.join();
Holger
  • 285,553
  • 42
  • 434
  • 765
0

What ever Holger said does make sense, but not in the problem I posted. I do agree that the question is not written in the best way.

The problem was that the order in which the futures were written were causing a consistent increase in time.

Ideally the order of the future should not matter as long as the code is written in a correct reactive fashion

The reason of the problem was the default ForkJoinPool of Java and Java uses this pool by default to run all the CompletableFutures. If I run all the CompletableFutues with a custom pool, I get almost the same time, irrespective of the order in which the future statements were written.

I still need to find what are the limitation of ForkJoinPool and find why my custom pool of 20 threads performs better.

I ll update my answer when I find the right reason.

Robin
  • 6,879
  • 7
  • 37
  • 35