msg93895 - (view) |
Author: albert hofkamp (aioryi) |
Date: 2009-10-12 12:43 |
Current implementation (r71564) uses "'%s\n%s' % (old_val, new_line)" to merge multi-line options into one string. For options with many lines, this wastes a lot of CPU power. Attached patch against r71564 fixes this problem by first building a list of lines for each loaded option, and after reading the whole file, merging them with the already loaded data. In that way, the '\n'.join() can be performed once. Patched ConfigParser.py works against test/test_cfgparser.py (and Python 2.5) We have witnessed a reduction from 4 hours to 3 seconds loading time with Python 2.6 and an option of 800000 lines. |
|
|
msg111399 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2010-07-23 23:46 |
Won't happen for Python 2.x but the idea is good. The original patch won't apply for Py3k so I implemented this approach from scratch for Python 3.2. Patch included. Some microbenchmarks of mine show: - a 3-5% slowdown for very small files (we can ignore this since it's still a blink of an eye) - a 10% speedup in usual cases (files the size of a typical Pylons app configuration or a typical buildout) - more so in unrealistic ones (many values with > 100 lines each) Brett, a quick summary of changes: - the idea to introduce a list of lines instead of gluing '%s\n%s' during file reading was introduced - this approach needs an additional phase of joining the lists to strings after reading is done. - I used itertools.chain to add the default section for iteration purposes without having to create a separate list. I hope the additional dependency on itertools is no problem. - I reformatted a bit the changes by fred.drake in r78233 (he added support for valueless keys) - some other slight formatting changes apply as well - in the tests I've added a testcase for a file that uses multi-line values heavily. The file is generated temporarily since it's 127kB in size. Creation takes a fraction of a second so shouldn't be noticeable. |
|
|
msg111495 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-24 18:02 |
@Łukasz: I patched the unit test file only and ran it and all tests passed, I assume that this behaviour is incorrect. |
|
|
msg111497 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2010-07-24 18:20 |
Thanks, Mark. The tests should pass for both the old version and the patched one. The change in the code involves performance, not functionality. I didn't want to set assertions that are time based so naturally the new test also runs on the old implementation. The thing is that it runs slower. |
|
|
msg111500 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-24 18:34 |
Łukasz in that case I think we should get this committed as the patch is small and looks clean. I assume that this will be backported as 2.7 and 3.1 are likely to be around for some years yet. |
|
|
msg111569 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2010-07-25 23:04 |
In I remarked the new itertools dependency. It seems it is in fact problematic because when building Python from scratch `setup.py` which is used to build C extensions is using configparser. And one of the C-only modules is itertools :) Attached a new patch that doesn't include itertools dependencies. The only difference with the last one is: 96d95 < import itertools 549,550c548,550 < all_sections = itertools.chain([self._defaults], < self._sections.values()) --- > all_sections = [self._defaults] > all_sections.extend(self._sections.values()) > |
|
|
msg111578 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2010-07-25 23:41 |
Patch from py3k root. Some minor formatting changes suggested by Ezio Melotti included. |
|
|
msg111590 - (view) |
Author: Brian Curtin (brian.curtin) *  |
Date: 2010-07-26 02:31 |
Committed to py3k in r83154 and release27-maint in r83155. |
|
|