Re: [Qemu-block] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once

2016-09-17 Thread Yaowei Bai
On Wed, Sep 14, 2016 at 05:40:17PM +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 14, 2016 at 07:03:39AM -0400, Yaowei Bai wrote:
> > As epoll whether enabled or not is a global setting, we can just
> > check it only once rather than checking it with every node iteration.
> > Through this we can avoid a lot of checks when epoll is not enabled.
> > 
> > Signed-off-by: Yaowei Bai 
> > Reviewed-by: Xiubo Li 
> > ---
> >  aio-posix.c | 12 +++-
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> The commit message says "improve aio_poll performance" but no benchmark
> results were provided.  Therefore I can't take this patch as a
> performance optimization.  I'm still happy to merge the patch since it
> makes the if statement simpler but I'll rename it to "aio-posix: avoid
> unnecessary aio_epoll_enabled() calls".

Sorry for replying late, i just came back from my vacation. And i'd like
to say it's ok for me to rename the commit message. Thank you for doing
it.

> 
> I don't think this patch gives a measurable performance improvement.  If

Yes, i agree with you at this point in consideration of there's no too many
fds to poll at most time.

> you believe otherwise, please post benchmark results.  Please let me
> know what you think.
> 
> Stefan







Re: [Qemu-block] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once

2016-09-14 Thread Stefan Hajnoczi
On Wed, Sep 14, 2016 at 07:03:39AM -0400, Yaowei Bai wrote:
> As epoll whether enabled or not is a global setting, we can just
> check it only once rather than checking it with every node iteration.
> Through this we can avoid a lot of checks when epoll is not enabled.
> 
> Signed-off-by: Yaowei Bai 
> Reviewed-by: Xiubo Li 
> ---
>  aio-posix.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)

The commit message says "improve aio_poll performance" but no benchmark
results were provided.  Therefore I can't take this patch as a
performance optimization.  I'm still happy to merge the patch since it
makes the if statement simpler but I'll rename it to "aio-posix: avoid
unnecessary aio_epoll_enabled() calls".

I don't think this patch gives a measurable performance improvement.  If
you believe otherwise, please post benchmark results.  Please let me
know what you think.

Stefan


signature.asc
Description: PGP signature


[Qemu-block] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once

2016-09-14 Thread Yaowei Bai
As epoll whether enabled or not is a global setting, we can just
check it only once rather than checking it with every node iteration.
Through this we can avoid a lot of checks when epoll is not enabled.

Signed-off-by: Yaowei Bai 
Reviewed-by: Xiubo Li 
---
 aio-posix.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/aio-posix.c b/aio-posix.c
index 43162a9..4ef34dd 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -431,11 +431,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
 assert(npfd == 0);
 
 /* fill pollfds */
-QLIST_FOREACH(node, &ctx->aio_handlers, node) {
-if (!node->deleted && node->pfd.events
-&& !aio_epoll_enabled(ctx)
-&& aio_node_check(ctx, node->is_external)) {
-add_pollfd(node);
+
+if (!aio_epoll_enabled(ctx)) {
+QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+if (!node->deleted && node->pfd.events
+&& aio_node_check(ctx, node->is_external)) {
+add_pollfd(node);
+}
 }
 }
 
-- 
1.8.3.1