You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Ernie Allen <ea...@redhat.com> on 2014/06/24 14:50:52 UTC
Review Request 22930: Calculate duration in AsyncIO after readCallback
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22930/
-----------------------------------------------------------
Review request for qpid, Andrew Stitcher and Gordon Sim.
Repository: qpid
Description
-------
The duration calculation in sys/posix/AsyncIO.cpp readable() was happening before the readCallback() call. This was allowing the loop to execute for more than the allowable time (as set by threadMaxIoTimeNs).
This patch just changes where the duration is calculated in readable() and writeable().
Diffs
-----
/trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1605066
Diff: https://reviews.apache.org/r/22930/diff/
Testing
-------
Ran reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=977869. The connection delay dropped by 19%.
Thanks,
Ernie Allen
Re: Review Request 22930: Calculate duration in AsyncIO after readCallback
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22930/#review46520
-----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp
<https://reviews.apache.org/r/22930/#comment81967>
Unnecessary and misleading comment.
The previous location was actually wrong, so this comment would be something like "calculate duration in correct place" which makes no real sense in a comment.
/trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp
<https://reviews.apache.org/r/22930/#comment81966>
I don't think the changes to the write side are necessary, the calculation for write time was already close enough to correct.
There are no callbacks or operations that take locks between calculating the duration here and any place that use it.
The duration is recalculated after the idle callback already.
- Andrew Stitcher
On June 24, 2014, 12:50 p.m., Ernie Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22930/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 12:50 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Gordon Sim.
>
>
> Repository: qpid
>
>
> Description
> -------
>
> The duration calculation in sys/posix/AsyncIO.cpp readable() was happening before the readCallback() call. This was allowing the loop to execute for more than the allowable time (as set by threadMaxIoTimeNs).
>
> This patch just changes where the duration is calculated in readable() and writeable().
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1605066
>
> Diff: https://reviews.apache.org/r/22930/diff/
>
>
> Testing
> -------
>
> Ran reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=977869. The connection delay dropped by 19%.
>
>
> Thanks,
>
> Ernie Allen
>
>
Re: Review Request 22930: Calculate duration in AsyncIO after readCallback
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22930/#review46553
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Stitcher
On June 24, 2014, 2:23 p.m., Ernie Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22930/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 2:23 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Gordon Sim.
>
>
> Repository: qpid
>
>
> Description
> -------
>
> The duration calculation in sys/posix/AsyncIO.cpp readable() was happening before the readCallback() call. This was allowing the loop to execute for more than the allowable time (as set by threadMaxIoTimeNs).
>
> This patch just changes where the duration is calculated in readable() and writeable().
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1605080
>
> Diff: https://reviews.apache.org/r/22930/diff/
>
>
> Testing
> -------
>
> Ran reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=977869. The connection delay dropped by 19%.
>
>
> Thanks,
>
> Ernie Allen
>
>
Re: Review Request 22930: Calculate duration in AsyncIO after readCallback
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22930/#review46527
-----------------------------------------------------------
Ship it!
/trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp
<https://reviews.apache.org/r/22930/#comment81971>
Minor nitpick: extra space here.
- Gordon Sim
On June 24, 2014, 2:23 p.m., Ernie Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22930/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 2:23 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Gordon Sim.
>
>
> Repository: qpid
>
>
> Description
> -------
>
> The duration calculation in sys/posix/AsyncIO.cpp readable() was happening before the readCallback() call. This was allowing the loop to execute for more than the allowable time (as set by threadMaxIoTimeNs).
>
> This patch just changes where the duration is calculated in readable() and writeable().
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1605080
>
> Diff: https://reviews.apache.org/r/22930/diff/
>
>
> Testing
> -------
>
> Ran reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=977869. The connection delay dropped by 19%.
>
>
> Thanks,
>
> Ernie Allen
>
>
Re: Review Request 22930: Calculate duration in AsyncIO after readCallback
Posted by Ernie Allen <ea...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22930/
-----------------------------------------------------------
(Updated June 24, 2014, 2:23 p.m.)
Review request for qpid, Andrew Stitcher and Gordon Sim.
Changes
-------
Incorporated Andrew's suggested changes.
Repository: qpid
Description
-------
The duration calculation in sys/posix/AsyncIO.cpp readable() was happening before the readCallback() call. This was allowing the loop to execute for more than the allowable time (as set by threadMaxIoTimeNs).
This patch just changes where the duration is calculated in readable() and writeable().
Diffs (updated)
-----
/trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1605080
Diff: https://reviews.apache.org/r/22930/diff/
Testing
-------
Ran reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=977869. The connection delay dropped by 19%.
Thanks,
Ernie Allen