apache
LANG-1747: Measure the execution time of functional interfaces
https://issues.apache.org/jira/browse/LANG-1747
The StopWatch is now able to take the execution time for the execution of an implementation of the following interfaces:
java.util.function.BiConsumerjava.util.function.BiFunctionjava.util.function.BiPredicatejava.util.function.Consumerjava.util.function.Functionjava.util.function.Predicatejava.util.function.Supplierorg.apache.commons.lang3.function.FailableBiConsumerorg.apache.commons.lang3.function.FailableBiFunctionorg.apache.commons.lang3.function.FailableBiPredicateorg.apache.commons.lang3.function.FailableConsumerorg.apache.commons.lang3.function.FailableFunctionorg.apache.commons.lang3.function.FailablePredicateorg.apache.commons.lang3.function.FailableRunnableorg.apache.commons.lang3.function.FailableSupplierorg.apache.commons.lang3.function.TriConsumerorg.apache.commons.lang3.function.TriFunction
@garydgregory Could you please review the first draft, so that I could have a early feedback before I continue?
Out of all of these new APIs, which ones do you actually need?
Let's not add every functional interface unless they are actually fulfill a use case
Use failable versions
All of them. Over the years I hat to measure some of them for statistics or debugging. So my decision was to provide an API which does not only covers my needs just in this monthm, but supports all standard interfaces in the JDK.
After looking some more at this PR, all the new methods "add" a duration to a stopwatch, which makes me wonder (as discussed on the mailing list), if all the new methods should be called "add". Food for thought...
Yes, I think we should consider this. add, takeTime or measure. But we should keep in mind, that the actual measurement is done during the execution of some of these methods. For example test(Predicate ..) does not measure any time at the moment of the method call. It returns only a wrapped instance, which will measure the time if executed once or multiple times.
What do you think?
Hello @obfischer
- The tests don't assert that the stopwatch measured time.
- Another assertion would be that the lambda asserts that the stopwatch is running while it is invoked. For example:
assertEquals("Foobar", watch.get((FailableSupplier<String, Throwable>) () -> {
assertTrue(watch.isStarted());
return "Foobar";
}));
- I don't understand your comment about
test(Predicate..), what it supposed to betest(Predicate)or did you mean a vararg version of the APItest(Predicate...)? What "wrapping" are you talking about? All the new methods don't "wrap" anything in the composition sense, they do delegate to the given lambda while managing the stopwatch state before and after the delegation. I must be missing something ;-)
Hello @obfischer
* The tests don't assert that the stopwatch measured time. * Another assertion would be that the lambda asserts that the stopwatch is running while it is invoked. For example:assertEquals("Foobar", watch.get((FailableSupplier<String, Throwable>) () -> { assertTrue(watch.isStarted()); return "Foobar"; }));* I don't understand your comment about `test(Predicate..)`, what it supposed to be `test(Predicate)` or did you mean a vararg version of the API `test(Predicate...)`? What "wrapping" are you talking about? All the new methods don't "wrap" anything in the composition sense, they do delegate to the given lambda while managing the stopwatch state before and after the delegation. I must be missing something ;-)
Hi @garydgregory, could I use Mockito to verify that the measurement is done properly?
@obfischer
Sure, I don't see why not.
@garydgregory The project uses already EasyMock. I would, because I am lazy, prefer Mockito. Is it still fine to use Mockito or should I start to learn EasyMock= 😄
I don't see why mocking is needed in the first place since the stopwatch state change can be asserted from the lambda, but let's see.
Hi @garydgregory I am very busy at the moment. I will continue to work on it in some days...
@obfischer
Run mvn by itself before you push to catch all builds failures.
@obfischer Run
mvnby itself to run all build checks.
No, my ping was for my last comment on the implementation of resumeOrStartStopWatch. I didn't change any line of code until now.
@obfischer Run
mvnby itself to run all build checks.No, my ping was for my last comment on the implementation of
resumeOrStartStopWatch. I didn't change any line of code until now.
Hello @obfischer Please make sure the build is green. I'll review again then.
-1 as is, this is now deeply confusing, the functional methods should actually perform the measurement, not return a functional object that the user must then invoke to perform the measurement. Each time a functional method is called, the "start or resume" method is called but this measures time whether or the returned functional object is used or not. The functional methods should return the same value as their arguments, or void for the functional interfaces like consumers. I'll look over in more details over the next day or two.
@obfischer
I took a simpler route than we previously discussed by only adding StopWatch.run([Failable]Runnable) and
get([Failable]Supplier) which I believe should be able to handle the most common use cases.
You are credited in changes.xml.
If you there are some missing use cases, let's discuss.