Skip to content

Commit

Permalink
save_ts: use list instead of str concat. BZ 1600383
Browse files Browse the repository at this point in the history
Instead of accumulating a long string object, build up a list and join
it at the end into a string.

Even though modern Pythons are optimized for such string concatenations,
we're hitting some edge cases in Anaconda where the save_ts() method
becomes really slow when dumping large transactions (~80 seconds for a
"Server with GUI" selection including all Add-Ons on RHEL-7.6).

Using the list paradigm here (which is a good practice[1] anyway)
reduces that time to ~20 seconds for the same transaction.

Of course, the proper fix should land in Anaconda/Python[2] eventually
but let's do this as a hotfix for now.

[1] https://docs.python.org/3/faq/programming.html#what-is-the-most-efficient-way-to-concatenate-many-strings-together
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1674016
  • Loading branch information
dmnks committed Mar 26, 2019
1 parent d97a853 commit f9e636d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 30 deletions.
18 changes: 9 additions & 9 deletions yum/__init__.py
Expand Up @@ -6903,24 +6903,24 @@ def save_ts(self, filename=None, auto=False):

self._ts_save_file = filename

msg = "%s\n" % self.rpmdb.simpleVersion(main_only=True)[0]
msg += "%s\n" % self.ts.getTsFlags()
msg = ["%s\n" % self.rpmdb.simpleVersion(main_only=True)[0],
"%s\n" % self.ts.getTsFlags()]

if self.tsInfo._pkgSack is None: # Transactions have pkgSack?
msg += "1\n"
msg += ["1\n"]
else:
msg += "%s\n" % (len(self.repos.listEnabled()) + 1)
msg += ["%s\n" % (len(self.repos.listEnabled()) + 1)]
for r in self.repos.listEnabled():
msg += "%s:%s:%s\n" % (r.id, len(r.sack), r.repoXML.revision)
msg += ["%s:%s:%s\n" % (r.id, len(r.sack), r.repoXML.revision)]

# Save what we think the future rpmdbv will be.
msg += "%s:%s\n" % ('installed', self.tsInfo.futureRpmDBVersion())
msg += ["%s:%s\n" % ('installed', self.tsInfo.futureRpmDBVersion())]

msg += "%s\n" % len(self.tsInfo.getMembers())
msg += ["%s\n" % len(self.tsInfo.getMembers())]
for txmbr in self.tsInfo.getMembers():
msg += txmbr._dump()
msg += [txmbr._dump()]
try:
f.write(msg)
f.write(''.join(msg))
f.close()
except (IOError, OSError), e:
self._ts_save_file = None
Expand Down
45 changes: 24 additions & 21 deletions yum/transactioninfo.py
Expand Up @@ -873,44 +873,47 @@ def __repr__(self):
return "<%s : %s (%s)>" % (self.__class__.__name__, str(self),hex(id(self)))

def _dump(self):
msg = "mbr: %s,%s,%s,%s,%s %s\n" % (self.name, self.arch, self.epoch,
self.version, self.release, self.current_state)
msg += " repo: %s\n" % self.po.repo.id
msg += " ts_state: %s\n" % self.ts_state
msg += " output_state: %s\n" % self.output_state
msg += " isDep: %s\n" % bool(self.isDep)
msg += " reason: %s\n" % self.reason
#msg += " process: %s\n" % self.process
msg += " reinstall: %s\n" % bool(self.reinstall)
msg = ["mbr: %s,%s,%s,%s,%s %s\n" %
(self.name, self.arch, self.epoch, self.version, self.release,
self.current_state),
" repo: %s\n" % self.po.repo.id,
" ts_state: %s\n" % self.ts_state,
" output_state: %s\n" % self.output_state,
" isDep: %s\n" % bool(self.isDep),
" reason: %s\n" % self.reason,
# " process: %s\n" % self.process,
" reinstall: %s\n" % bool(self.reinstall)]

if self.relatedto:
msg += " relatedto:"
msg += [" relatedto:"]
for (po, rel) in self.relatedto:
pkgorigin = 'a'
if isinstance(po, YumInstalledPackage):
pkgorigin = 'i'
msg += " %s,%s,%s,%s,%s@%s:%s" % (po.name, po.arch, po.epoch,
po.version, po.release, pkgorigin, rel)
msg += "\n"
msg += [" %s,%s,%s,%s,%s@%s:%s" %
(po.name, po.arch, po.epoch, po.version, po.release,
pkgorigin, rel)]
msg += ["\n"]

for lst in ['depends_on', 'obsoletes', 'obsoleted_by', 'downgrades',
'downgraded_by', 'updates', 'updated_by']:
thislist = getattr(self, lst)
if thislist:
msg += " %s:" % lst
msg += [" %s:" % lst]
for po in thislist:
pkgorigin = 'a'
if isinstance(po, YumInstalledPackage):
pkgorigin = 'i'
msg += " %s,%s,%s,%s,%s@%s" % (po.name, po.arch, po.epoch,
po.version, po.release, pkgorigin)
msg += "\n"
msg += [" %s,%s,%s,%s,%s@%s" %
(po.name, po.arch, po.epoch, po.version,
po.release, pkgorigin)]
msg += ["\n"]

if self.groups:
msg += " groups: %s\n" % ' '.join(self.groups)
msg += [" groups: %s\n" % ' '.join(self.groups)]
if self.environments:
msg += " environments: %s\n" % ' '.join(self.environments)
msg += [" environments: %s\n" % ' '.join(self.environments)]
if self.repopkg:
msg += " repopkg: %s\n" % self.repopkg
msg += [" repopkg: %s\n" % self.repopkg]

return msg
return ''.join(msg)

0 comments on commit f9e636d

Please sign in to comment.