Dan Kenigsberg has posted comments on this change.
Change subject: Optimize netinfo.py:get()
......................................................................
Patch Set 2:
(3 comments)
....................................................
File lib/vdsm/ipwrapper.py
Line 230: """Returns a list of Link objects for each link in the system."""
Line 231: return [Link.fromText(line) for line in linksShowDetailed()]
Line 232:
Line 233:
Line 234: def getVisibleLinks():
personally, I am not crazy for once-used helper functions becoming public. I
have a naming issue: we have a concept of "hidden", and now have to think of
it's opposite, too.
Line 235: for link in getLinks():
Line 236: if not link.isHidden():
Line 237: yield link
Line 238:
....................................................
File lib/vdsm/netinfo.py
Line 540:
netAttr.get('qosOutbound'))
Line 541: except KeyError:
Line 542: continue # Do not report missing libvirt networks.
Line 543:
Line 544: for dev in getVisibleLinks():
I fail to see the sqrt() optimization. Formerly, we had 4 iteration of n
devices; now we have 1 iteration with 4 branches. Could you explain where is
the big gain hiding from me?
Line 545: if dev.isBRIDGE():
Line 546: d['bridges'][dev.name] = \
Line 547: _bridgeinfo(dev.name, gateways, ipv6routes)
Line 548: if dev.isNICLike():
Line 544: for dev in getVisibleLinks():
Line 545: if dev.isBRIDGE():
Line 546: d['bridges'][dev.name] = \
Line 547: _bridgeinfo(dev.name, gateways, ipv6routes)
Line 548: if dev.isNICLike():
Wouldn't "elif" be more fitting?
Line 549: d['nics'][dev.name] = _nicinfo(dev.name, paddr)
Line 550: if dev.isBOND():
Line 551: d['bondings'][dev.name] = _bondinfo(dev.name)
Line 552: if dev.isVLAN():
--
To view, visit http://gerrit.ovirt.org/21652
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I70379b24084fa8bec88425de9eca106e2a6ae8f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Assaf Muller <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
