On Mon, Sep 28, 2015 at 12:14:02PM -0400, Laine Stump wrote:
On 09/28/2015 05:27 AM, Daniel P. Berrange wrote:
>On Fri, Sep 25, 2015 at 12:41:23PM -0700, David Lutterkort wrote:
>>On Fri, Sep 25, 2015 at 11:35 AM, Laine Stump <laine(a)laine.org> wrote:
>>
>>>On 09/25/2015 01:27 PM, Daniel P. Berrange wrote:
>>>
>>>>So, I instrumented the netcf and augeas code to checking timings.
>>>>
>>>>The aug_get calls time less than a millisecond, as do the various
>>>>other calls. I found the bulk of the time is actually coming from
>>>>the netcf function "get_augeas", which in turns call
"aug_load"
>>>>for every single damn netcf function call. So when we have 500
>>>>interfaces, we're telling augeas to load all the config files
>>>>1000 times. That's where the slowness is coming from....
>>>>
>>>>Either we need to stop loading config files on every fnuction
>>>>call in netcf,
>>>>
>>>Right you are! netcf has a variable "load_augeas" that is set each
time an
>>>API call is started. I removed the line that sets it, and the time for
>>>virsh iface-list --all for 309 toplevel interfaces (300 vlans connected to
>>>300 bridges + whatever is really on the host) went from 22.2 sec. to 1.47
>>>sec!
>>>
>>Strange .. that variable should just control whether we call aug_load or
>>not, it shouldn't affect whether we actual create a new augeas instance
>>(which indeed can be very expensive)
>>
>>I see that we pass AUG_NO_MODL_AUTOLOAD to aug_init - it should not be
>>necessary, but you might want to try quickly if passing
>>AUG_NO_MODL_AUTOLOAD|AUG_NO_LOAD changes things (it shouldn't)
>>
>>
>>>But of course we can't blindly re-use the initial data forever. Looking
at
>>>aug_load() itself, I see a lot of comments about everything it does to
>>>avoid unnecessary re-loading of files. This was added in 2010, and is
>>>probably what I remember David talking about:
>>>
>>>commit 5ee8163051be8214507c13c86171ac90ca7cb91f
>>>Author: David Lutterkort <lutter(a)redhat.com>
>>>Date: Tue Jun 29 15:32:44 2010 -0700
>>>
>>> Avoid unnecessary file parsing when reloading the tree
>>>
>>> We used to reparse every file we knew about upon aug_load. Now, we only
>>> reparse files if the file has changed on disk.
>>>
>>> We test a few scenarios to make sure aug_load retains its behavior of
>>> obliterating the tree and filling it with the latest from disk. This
>>> includes throwing away unsaved changes or trees that have been deleted.
>>>
>>>So now the question is whether something can be done to improve aug_load()
>>>(maybe it used to perform better and there has been a regression?), or if
>>>netcf should derive a list of files from the list sent to augeas, and do
>>>its own checking of the timestamps.
>>>
>>What should happen when you call aug_load is that augeas goes and stats all
>>the files it knows about and if their mtime hasn't changed since we last
>>loaded them, that should be the end of it. I have a hard time believing
>>that stating 309 files takes ~ 1.5s though - seems there's something else
>>going on. Maybe there are still some cases in which a new augeas instance
>>gets created ?
>Oh, you are mis-reading the numbers that Laine gives here. He has 310
>interfaces, which corresponds to 620 API calls into netcf, which is
>620 cals to aug_load() in 22 seconds. So each aug_load() is 35 milliseconds.
>
>I've just tested on my machine with SSD and 500 interfaces. I get 25 ms
>for each call to aug_load().
>
>strace'ing it, I confirm that is really is only doing stat() on a whole
>bunch of files.
>
>So really the problem we have here is that we are doing O(n^2) calls to
>aug_load() where n == number of interfaces.
>
>So we really must change what we do so that we only do 1 call to aug_load
>in order to list interfaces. I'm thinking the simplest way is to just
>define a new API in augeas to bulk query the interfaces + their macs
>in a single call.
I think you mean a new netcf API :-)
How about this as an alternative: the "load_augeas" flag in netcf could
instead be a timestamp, and we would only reload the augeas tree if it had
been at least [some very small number still much greater than 35msec] had
passed since the last time it was loaded.
Doing it that way we could avoid adding complexity to the API for a single
use case, and we would also be able to easily backport it to downstream
releases of earlier versions of netcf (e.g. RHEL/CentOS 6)
Yeah, I guess we could easily say only refresh once per second.
Regards,
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|