[PATCH 3/3] Minimize number of times storage.partitions is accessed. (#1155984)
David Lehman
dlehman at redhat.com
Fri Sep 11 20:46:54 UTC 2015
On Fri, 2015-09-11 at 16:35 -0400, Samantha N. Bueno wrote:
> On Fri, Aug 21, 2015 at 12:56:41PM -0500, David Lehman wrote:
> > On 08/21/2015 12:25 PM, Samantha N. Bueno wrote:
> > > This is also a non-trivial culprit in causing disk partitioning
> > > to seem
> > > slow. Every time storage.partitions is accessed,
> > > getDevicesByInstance()
> > > is called, which, collectively, had significant overhead with
> > > high
> > > numbers of disks.
> > >
> > > Resolves: rhbz#1155984
> > > ---
> > > blivet/partitioning.py | 17 +++++++++--------
> > > 1 file changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> > > index f0a9a7a..c6c0ca5 100644
> > > --- a/blivet/partitioning.py
> > > +++ b/blivet/partitioning.py
> > > @@ -860,11 +860,12 @@ def updateExtendedPartitions(storage,
> > > disks):
> > > """
> > > # XXX hack -- if we created any extended partitions we need
> > > to add
> > > # them to the tree now
> > > + partitions = storage.partitions
> > > for disk in disks:
> > > extended = disk.format.extendedPartition
> > > if not extended:
> > > # remove any obsolete extended partitions
> > > - for part in storage.partitions:
> > > + for part in partitions:
> > > if part.disk == disk and part.isExtended:
> > > if part.exists:
> > > storage.destroyDevice(part)
> > > @@ -880,7 +881,7 @@ def updateExtendedPartitions(storage, disks):
> > > device.partedPartition = extended
> > >
> > > # remove any obsolete extended partitions
> > > - for part in storage.partitions:
> > > + for part in partitions:
> > > if part.disk == disk and part.isExtended and \
> > > part.partedPartition not in
> > > disk.format.partitions:
> > > if part.exists:
> > > @@ -929,13 +930,13 @@ def doPartitioning(storage):
> > > # XXX This does not remove the extended from the
> > > parted.Disk, but it should
> > > # cause removeNewPartitions to remove it since there
> > > will no longer be
> > > # a PartitionDevice for it.
> > > - for partition in storage.partitions:
> > > + partitions = storage.partitions
> > > + for partition in partitions:
> > > if not partition.exists and partition.isExtended and \
> > > not storage.devicetree.findActions(device=partition,
> > > action_type="create"):
> > > storage.devicetree._removeDevice(partition,
> > > modparent=False, force=True)
> >
> > Since you might be removing a partition from the list here you
> > should
> > also remove it from 'partitions' at the same time, which will mean
> > you
> > should iterate over a slice/copy of partitions in the above loop.
> >
> > In the same spirit, please make sure nothing is done in between the
> > rest of these that might change storage.partitions.
>
> Now that you've mentioned this, I'm a bit wary to push this patch. It
> doesn't seem to have as big an impact to the time lag as I thought,
> so I
> think I am going to leave this out of the set for now if that's all
> right with you.
/me makes taunting chicken sounds...
You could at least keep this one:
diff --git a/blivet/partitioning.py b/blivet/partitioning.py
index 6505825..a0ae641 100644
--- a/blivet/partitioning.py
+++ b/blivet/partitioning.py
@@ -539,7 +539,7 @@ def doPartitioning(storage):
storage.devicetree._removeDevice(partition,
modparent=False, force=True)
partitions = storage.partitions[:]
- for part in storage.partitions:
+ for part in partitions:
part.req_bootable = False
if not part.exists:
# start over with flexible-size requests
David
>
> Samantha
>
> > David
> >
> > >
> > > - partitions = storage.partitions[:]
> > > - for part in storage.partitions:
> > > + for part in partitions:
> > > part.req_bootable = False
> > > if not part.exists:
> > > # start over with flexible-size requests
> > > @@ -956,7 +957,7 @@ def doPartitioning(storage):
> > > raise
> > > else:
> > > # Mark all growable requests as no longer growable.
> > > - for partition in storage.partitions:
> > > + for partition in partitions:
> > > log.debug("fixing size of %s", partition)
> > > partition.req_grow = False
> > > partition.req_base_size = partition.size
> > > @@ -968,7 +969,7 @@ def doPartitioning(storage):
> > > # The number and thus the name of partitions may have
> > > changed now,
> > > # allocatePartitions() takes care of this for new
> > > partitions, but not
> > > # for pre-existing ones, so we update the name of all
> > > partitions here
> > > - for part in storage.partitions:
> > > + for part in partitions:
> > > # leave extended partitions as-is -- we'll handle
> > > them separately
> > > if part.isExtended:
> > > continue
> > > @@ -976,7 +977,7 @@ def doPartitioning(storage):
> > >
> > > updateExtendedPartitions(storage, disks)
> > >
> > > - for part in [p for p in storage.partitions if not
> > > p.exists]:
> > > + for part in [p for p in partitions if not p.exists]:
> > > problem = part.checkSize()
> > > if problem < 0:
> > > raise PartitioningError(_("partition is too
> > > small for %(format)s formatting "
> > >
> >
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
More information about the anaconda-patches
mailing list