You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Dominic Hamon <dh...@twopensource.com> on 2014/05/02 19:52:48 UTC
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/
-----------------------------------------------------------
(Updated May 2, 2014, 10:52 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
added issue ref
Bugs: MESOS-1284
https://issues.apache.org/jira/browse/MESOS-1284
Repository: mesos-git
Description
-------
see summary
Diffs
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 26f997c0c5e034dab88cf59bb641754fec7b5abe
3rdparty/libprocess/src/tests/metrics_tests.cpp 75f1f0e35fdb1ec707785d1ebb903af99551bdaa
Diff: https://reviews.apache.org/r/20918/diff/
Testing
-------
make check
Thanks,
Dominic Hamon
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Vinod Kone <vi...@gmail.com>.
> On May 21, 2014, 10:53 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/include/process/metrics/timer.hpp, line 65
> > <https://reviews.apache.org/r/20918/diff/2/?file=582655#file582655line65>
> >
> > Why is this pulled out of the locked section? Just do s/stop/Clock::now()/ below?
>
> Dominic Hamon wrote:
> The stop time should be the time 'stop' is called, not the time the lock is acquired.
just to be sure, is this a change in semantics in addition to swapping clock with stopwatch? afaict, in the stopwatch version the elapsed time was calculated after the lock was acquired.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/#review43663
-----------------------------------------------------------
On May 22, 2014, 5:30 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20918/
> -----------------------------------------------------------
>
> (Updated May 22, 2014, 5:30 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1284
> https://issues.apache.org/jira/browse/MESOS-1284
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
> 3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
>
> Diff: https://reviews.apache.org/r/20918/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Dominic Hamon <dh...@twopensource.com>.
> On May 21, 2014, 3:53 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/include/process/metrics/timer.hpp, line 65
> > <https://reviews.apache.org/r/20918/diff/2/?file=582655#file582655line65>
> >
> > Why is this pulled out of the locked section? Just do s/stop/Clock::now()/ below?
The stop time should be the time 'stop' is called, not the time the lock is acquired.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/#review43663
-----------------------------------------------------------
On May 22, 2014, 10:30 a.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20918/
> -----------------------------------------------------------
>
> (Updated May 22, 2014, 10:30 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1284
> https://issues.apache.org/jira/browse/MESOS-1284
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
> 3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
>
> Diff: https://reviews.apache.org/r/20918/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Dominic Hamon <dh...@twopensource.com>.
> On May 21, 2014, 3:53 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/include/process/metrics/timer.hpp, line 65
> > <https://reviews.apache.org/r/20918/diff/2/?file=582655#file582655line65>
> >
> > Why is this pulled out of the locked section? Just do s/stop/Clock::now()/ below?
>
> Dominic Hamon wrote:
> The stop time should be the time 'stop' is called, not the time the lock is acquired.
>
> Vinod Kone wrote:
> just to be sure, is this a change in semantics in addition to swapping clock with stopwatch? afaict, in the stopwatch version the elapsed time was calculated after the lock was acquired.
it is for start/stop. in the time/_time construct we always stopped the stopwatch outside the lock. Being inside the lock was actually a bug.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/#review43663
-----------------------------------------------------------
On May 22, 2014, 10:30 a.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20918/
> -----------------------------------------------------------
>
> (Updated May 22, 2014, 10:30 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1284
> https://issues.apache.org/jira/browse/MESOS-1284
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
> 3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
>
> Diff: https://reviews.apache.org/r/20918/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/#review43663
-----------------------------------------------------------
3rdparty/libprocess/include/process/metrics/timer.hpp
<https://reviews.apache.org/r/20918/#comment77953>
s/stopwatch/Clock/
here and everywhere else.
3rdparty/libprocess/include/process/metrics/timer.hpp
<https://reviews.apache.org/r/20918/#comment77954>
Why is this pulled out of the locked section? Just do s/stop/Clock::now()/ below?
3rdparty/libprocess/src/tests/metrics_tests.cpp
<https://reviews.apache.org/r/20918/#comment77955>
yay!
3rdparty/libprocess/src/tests/metrics_tests.cpp
<https://reviews.apache.org/r/20918/#comment77956>
I'm confused on what this is trying to test. Maybe some comments are in order?
- Vinod Kone
On May 15, 2014, 5:43 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20918/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 5:43 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1284
> https://issues.apache.org/jira/browse/MESOS-1284
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
> 3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
>
> Diff: https://reviews.apache.org/r/20918/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/#review44002
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On May 22, 2014, 5:30 p.m., Dominic Hamon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20918/
> -----------------------------------------------------------
>
> (Updated May 22, 2014, 5:30 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1284
> https://issues.apache.org/jira/browse/MESOS-1284
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
> 3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
>
> Diff: https://reviews.apache.org/r/20918/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Dominic Hamon
>
>
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/
-----------------------------------------------------------
(Updated May 22, 2014, 10:30 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1284
https://issues.apache.org/jira/browse/MESOS-1284
Repository: mesos-git
Description
-------
see summary
Diffs (updated)
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
Diff: https://reviews.apache.org/r/20918/diff/
Testing
-------
make check
Thanks,
Dominic Hamon
Re: Review Request 20918: Timer now uses Clock instead of Stopwatch to
enable easier testing.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20918/
-----------------------------------------------------------
(Updated May 15, 2014, 10:43 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
rebased
Bugs: MESOS-1284
https://issues.apache.org/jira/browse/MESOS-1284
Repository: mesos-git
Description
-------
see summary
Diffs (updated)
-----
3rdparty/libprocess/include/process/metrics/timer.hpp 9236954425354a2d88eaeace353722ba9b40fbd5
3rdparty/libprocess/src/tests/metrics_tests.cpp 8a131218b858a6571bd520c470eac850a3fd345e
Diff: https://reviews.apache.org/r/20918/diff/
Testing
-------
make check
Thanks,
Dominic Hamon