Re: [libvirt] [RFC] Substitute poll by epoll in virEventPollRunOnce
On Fri, Feb 10, 2017 at 04:29:27PM +0300, Derbyshev Dmitriy wrote:
From: Derbyshev Dmitry <dderbyshev(a)virtuozzo.com>
This makes it possible to avoid allocations in
virEventPollMakePollFDs, as well as generally reduces time spent in
kernel if handles remain the same, which should generally be true.
Yep, there's no question that epoll is a clear winner in terms of
performance vs poll(). So from that POV, i'd totally welcome
epoll support in libvirt.
Questions:
1) What to do with probe in virEventPollRunOnce?
Probes are not considered part of the long term public ABI / API
stability guarantee. They are an adhoc mechanism for debugging
/ troubleshooting, so free to be changed at will. We try to
avoid breaking where possible, but if needed we can certainly
do it.
2) Are ifdef an acceptable solution to epoll avaliability issues?
There are quite a large number of ifdefs in your patch below.
I think it could be worth having a separate vireventepoll.c
for the epoll impl. Then choose to compile vireventpoll.c vs
vireventepoll.c at configure time. We could have vireventcommon.c
if there is some portion of code that can be shared.
3) Is using MAX_POLL_EVENTS_AT_ONCE constant a valid solution?
What is that controlling ? Is that the maximum number of
FDs that are monitored, or the maximum number of events
that can be reported at once ? What is the behaviour
if the limit is reached / exceeded ?
4) some fds are sometimes added twice, but only once with
events!=0? This complicates virEventPoll*Handle fuctions.
At the very least, it is called twice in
_dbus_watch_list_set_functions.
If libdbus does that there's nothing we can really do to
prevent it. The only other option would be to move the
extra complexity into src/util/virdbus.c instead, so that
it is guaranteed to only register each FD once, and will
multiplex / demultiplex as needed. I'm not sure that will
be any easier
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|