Re: [Zope-dev] SVN: Zope/branches/2.13/ fix `LazyMap` to avoid unnecessary function calls when not accessing items in order (fixes http://dev.plone.org/plone/ticket/9018)
hi tres, On 14.12.2010, at 18:11, Tres Seaver wrote: On 12/14/2010 09:12 AM, Andreas Zeidler wrote: Modified: Zope/branches/2.13/src/Products/ZCatalog/Lazy.py === --- Zope/branches/2.13/src/Products/ZCatalog/Lazy.py 2010-12-14 13:24:24 UTC (rev 118862) +++ Zope/branches/2.13/src/Products/ZCatalog/Lazy.py 2010-12-14 14:12:11 UTC (rev 118863) @@ -145,44 +145,28 @@ # Don't access data until necessary def __init__(self, func, seq, length=None): -self._seq = seq -self._data = [] -self._func = func +self._seq=seq +self._func=func if length is not None: -self._len = length +self._len=length Please don't un-PEP8 a cleaned-up module: leave spaces around operators. If that was unintentional (maybe you pasted from a copy of a file made before the module was cleaned up?), then you still need to review the diff and edit out unintended changes first. yes, in was indeed unintentional, or rather unfortunate. i've originally worked on the 2.12 branch (in a plone 4 buildout) where i think hanno's pep8-related changes haven't been applied/back-ported yet. after that i pasted the version into 2.13 (and yes, the commit order was the other way round :)) and was in fact wondering about the longer diff when hanno (who was actually sitting next to me) pointed out he had changed the one-line if: and else: expressions on that branch. however, my alias for svn diff tells diff to not show white space changes so i missed the now again removed spaces around the =. anyway, it's fixed in c118895. else: self._len = len(seq) +self._marker = object() +self._data = [self._marker] * self._len Have you measured the impact of pre-allocating here for very large sequences? i had not, but have now. turns out the allocation makes a difference earlier than i would have expected: accessing the first 20 items of the map is slower with the new version starting at sequences of a little more than 2000 items. and of course it (relatively) gets slower the longer the sequence is. setting up a 10^5 empty sequence takes about 0.5 ms on my machine. I'm *sure* that is a not a good trade-off for all cases: the catalog is often used for queries which return result sets in the order of 10^5 or more objects, and *very* rarely is it accessed randomly (I had never seen it before reading the bug entry you cite). The normal case is to take the first few entries (batch_size * batch_number) from the huge set. true, but the fix wasn't only about this use-case. it's mainly about avoiding extra function calls when fetching a batch other than the first. so far the map function wall called for every item up to the one accessed, e.g. for the 5th batch of 20 items each, you'd end up with 80 unnecessary (and possible expensive) calls. or iow, the old version wasn't really as lazy as it should have been… :) I think you would be better off finding a way to inject your style of LazyMap into the catalog for the random-access case, e.g., by passing '_lazy_map' into the methods you are using. i disagree. i think it should be fixed properly upstream. we should really avoid all that monkey-patching all over the place. but see below… BTW, another interesting alternative impplementation might be to use a dictionary instead of a list for 'data'. You could then avoid any pre-allocation at all. with the benchmark in place, i was curious and tried that as well: it beats the other versions independently of the sequence length and the number of accessed items, both sequentially and repeatedly over a single batch. in the benchmark they're accessed from the start, but skipping a few batches would make it even faster anyway. hence i've updated the code again in c118896. please let me know what you think, or if you want the test script i've been using. While we're at it, the 'try: ... except:' here is wasteful and slow. Lazy objects aren't persistent, and the '_seq' attribute is added unconditionally in '__init__'. you're right, this should be cleaned up as well. but since this isn't my code i was trying to keep the diff minimal (like on 2.12, i.e. in c118864, it obviously didn't work in the other one :)). in any case, it's also gone with c118896. cheers, andi -- pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 4.0 released! -- http://plone.org/4 PGP.sig Description: This is a digitally signed message part ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Zope Tests: 96 OK, 26 Failed
Summary of messages to the zope-tests list. Period Tue Dec 14 12:00:00 2010 UTC to Wed Dec 15 12:00:00 2010 UTC. There were 122 messages: 8 from Zope Tests, 4 from buildbot at pov.lt, 28 from buildbot at winbot.zope.org, 11 from ccomb at free.fr, 71 from jdriessen at thehealthagency.com. Test failures - Subject: FAILED : winbot / z3c.language.negotiator_py_265_32 From: buildbot at winbot.zope.org Date: Tue Dec 14 11:19:52 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026613.html Subject: FAILED : winbot / z3c.language.negotiator_py_265_32 From: buildbot at winbot.zope.org Date: Tue Dec 14 11:24:04 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026614.html Subject: FAILED : winbot / ztk_10 py_244_win32 From: buildbot at winbot.zope.org Date: Tue Dec 14 16:04:37 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026628.html Subject: FAILED : Zope Buildbot / zopetoolkit_win-py2.5 slave-win From: jdriessen at thehealthagency.com Date: Tue Dec 14 16:43:16 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026644.html Subject: FAILED : Zope Buildbot / zopetoolkit_win-py2.6 slave-win From: jdriessen at thehealthagency.com Date: Tue Dec 14 16:43:48 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026645.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.5 slave-ubuntu64 From: jdriessen at thehealthagency.com Date: Tue Dec 14 16:44:37 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026646.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.6 slave-ubuntu64 From: jdriessen at thehealthagency.com Date: Tue Dec 14 16:49:41 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026647.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.6 slave-ubuntu64 From: jdriessen at thehealthagency.com Date: Tue Dec 14 17:01:00 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026649.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.5 slave-ubuntu64 From: jdriessen at thehealthagency.com Date: Tue Dec 14 17:05:46 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026651.html Subject: FAILED : Zope Buildbot / zopetoolkit_win-py2.6 slave-win From: jdriessen at thehealthagency.com Date: Tue Dec 14 17:48:11 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026661.html Subject: FAILED : Zope Buildbot / zopetoolkit_win-py2.5 slave-win From: jdriessen at thehealthagency.com Date: Tue Dec 14 17:48:40 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026662.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.5 slave-ubuntu32 From: jdriessen at thehealthagency.com Date: Tue Dec 14 17:57:02 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026664.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.6 slave-ubuntu32 From: jdriessen at thehealthagency.com Date: Tue Dec 14 18:02:17 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026665.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.6 slave-ubuntu32 From: jdriessen at thehealthagency.com Date: Tue Dec 14 18:14:08 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026668.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.5 slave-ubuntu32 From: jdriessen at thehealthagency.com Date: Tue Dec 14 18:19:14 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026671.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.5 slave-osx From: jdriessen at thehealthagency.com Date: Tue Dec 14 18:54:57 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026677.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.6 slave-osx From: jdriessen at thehealthagency.com Date: Tue Dec 14 19:02:00 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026678.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.6 slave-osx From: jdriessen at thehealthagency.com Date: Tue Dec 14 19:17:26 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026679.html Subject: FAILED : Zope Buildbot / zopetoolkit-py2.5 slave-osx From: jdriessen at thehealthagency.com Date: Tue Dec 14 19:23:02 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026681.html Subject: FAILED : winbot / z3c.xmlhttp_py_265_32 From: buildbot at winbot.zope.org Date: Tue Dec 14 22:22:54 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026697.html Subject: FAILED : winbot / zope.broken_py_265_32 From: buildbot at winbot.zope.org Date: Tue Dec 14 23:01:05 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026698.html Subject: FAILED : winbot / z3c.ptcompat_py_265_32 From: buildbot at winbot.zope.org Date: Tue Dec 14 23:01:39 EST 2010 URL: http://mail.zope.org/pipermail/zope-tests/2010-December/026700.html Subject: FAILED : winbot / z3c.contents_py_265_32 From: buildbot at winbot.zope.org
[Zope-dev] zope.testbrowser and WebTest
Hi, I've committed a WebTest integration with testbrowser to a jinty-webtest branch. It basically uses WebTest as a backend to drive a WSGI application. This feature makes it possible to do this in a test: app = make_my_wsgi_app() from webtest import TestApp from zope.testbrowser.wsgi import Browser browser = Browser(TestApp(app)) And then use the browser as normal. I've managed to get the existing tests to run against this browser with two new testing dependencies: WebTest zope.app.wsgi If anyone objects to this feature, or wants to review it before I merge, please let me know. I will definitely fix the documentation and remaining test failure before merging. -- Brian Sutherland ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.testbrowser and WebTest
On Wed, Dec 15, 2010 at 8:06 AM, Brian Sutherland br...@vanguardistas.net wrote: I've committed a WebTest integration with testbrowser to a jinty-webtest branch. It basically uses WebTest as a backend to drive a WSGI application. This sounds like a nice improvement over using wsgi-intercept (http://code.google.com/p/wsgi-intercept/). I've taken a quick look at the branch. I saw a few of these in the diff: - from zope.testbrowser.testing import Browser +XXX: what to do with this? +XXX from zope.testbrowser.testing import Browser If you can give me some background maybe I can help with these. The copyright date in src/zope/testbrowser/wsgi.py needs the current year. I suspect large chunks of zope.testbrowser.wsgi can be eliminated with judicious refactoring. If you want, I should have time to review your branch again before you merge it. -- Benji York ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.testbrowser and WebTest
On Wed, Dec 15, 2010 at 8:06 AM, Brian Sutherland br...@vanguardistas.net wrote: Hi, I've committed a WebTest integration with testbrowser to a jinty-webtest branch. It basically uses WebTest as a backend to drive a WSGI application. This feature makes it possible to do this in a test: app = make_my_wsgi_app() from webtest import TestApp from zope.testbrowser.wsgi import Browser browser = Browser(TestApp(app)) And then use the browser as normal. I've managed to get the existing tests to run against this browser with two new testing dependencies: WebTest zope.app.wsgi If anyone objects to this feature, or wants to review it before I merge, please let me know. I will definitely fix the documentation and remaining test failure before merging. I'm not volunteering to review :), but this enhancement sounds great! Jim -- Jim Fulton ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.testbrowser and WebTest
On Wed, Dec 15, 2010 at 08:53:00AM -0500, Benji York wrote: On Wed, Dec 15, 2010 at 8:06 AM, Brian Sutherland br...@vanguardistas.net wrote: I've committed a WebTest integration with testbrowser to a jinty-webtest branch. It basically uses WebTest as a backend to drive a WSGI application. This sounds like a nice improvement over using wsgi-intercept (http://code.google.com/p/wsgi-intercept/). Exactly. I've been using that for a while, but it finally irritated me too much... I've taken a quick look at the branch. I saw a few of these in the diff: - from zope.testbrowser.testing import Browser +XXX: what to do with this? +XXX from zope.testbrowser.testing import Browser If you can give me some background maybe I can help with these. I now run the tests twice but with a different Browser variable which comes from the test setup. So I need to think of a way of changing the documentation so that it is still clear. The copyright date in src/zope/testbrowser/wsgi.py needs the current year. Sure, I'll fix that. I suspect large chunks of zope.testbrowser.wsgi can be eliminated with judicious refactoring. As you saw, I took the easy way out by copying testing.py. I wanted to see how easy it was to get the tests passing against WebTest. I'll correct my lazyness shortly ;) If you want, I should have time to review your branch again before you merge it. That'd be great. I'll do the refactoring and documentation fixed then ping you again. There's also a single remaining test failure that I may need your help with. -- Brian Sutherland ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.testbrowser and WebTest
On Wed, Dec 15, 2010 at 08:53:00AM -0500, Benji York wrote: If you want, I should have time to review your branch again before you merge it. Ok, I've done some minimal refactoring and am ready to merge the branch when you've reviewed it. -- Brian Sutherland ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.testbrowser and WebTest
On Wed, Dec 15, 2010 at 2:06 PM, Brian Sutherland br...@vanguardistas.net wrote: I've managed to get the existing tests to run against this browser with two new testing dependencies: WebTest zope.app.wsgi zope.app.wsgi shouldn't be a dependency of zope.testbrowser. It's ok if it's pulled in via a specific extra_requires like a [webtest] extra, though. Zope2 depends on zope.testbrowser and has no dependency on any zope.app packages - this must continue to be the case. Required test dependencies count towards real dependencies here. The real fix would probably be to move the reusable code out of zope.app.wsgi into a zope.wsgi package, but that might be more than you are willing to do now. I do believe some of the Grok people would be interested in this as well. Hanno ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.testbrowser and WebTest
On Thu, Dec 16, 2010 at 12:06:36AM +0100, Hanno Schlichting wrote:
On Wed, Dec 15, 2010 at 2:06 PM, Brian Sutherland
br...@vanguardistas.net wrote:
I've managed to get the existing tests to run against this browser with
two new testing dependencies:
WebTest
zope.app.wsgi
zope.app.wsgi shouldn't be a dependency of zope.testbrowser. It's ok
if it's pulled in via a specific extra_requires like a [webtest]
extra, though. Zope2 depends on zope.testbrowser and has no dependency
on any zope.app packages - this must continue to be the case. Required
test dependencies count towards real dependencies here.
I understand your point, but the situation was already pretty nasty as
zope.testbrowser already depended on:
zope.app.appsetup
zope.app.publication
zope.app.testing = 3.8
I agree that any extra dependency is not welcome, but this feature opens
up a path to radically reducing the dependencies of zope.testbrowser.
See below.
The real fix would probably be to move the reusable code out of
zope.app.wsgi into a zope.wsgi package, but that might be more than
you are willing to do now. I do believe some of the Grok people would
be interested in this as well.
There is also another route open now with the addition of the webtest
feature. We could invert the zope.testbrowser - zope.app.testing
dependency.
This is a major re-factoring, but will leave the zope.testbrowser
dependencies looking like this:
install_requires = [
'mechanize=0.2.0',
'setuptools',
'zope.interface',
'zope.schema',
'pytz',
],
extras_require = {
'test': [
'WebOb',
'WebTest',
]
Basically this would require:
* Re-writing the zope.testbrowser.ftests test application as a pure
WSGI app (using WebOb)
* Test only against zope.testbrowser.wsgi and refactor tests to not
use features from zope.app.testing.functional
* Move zope.testbrowser.testing into zope.testbrowser.wsgi and
zope.app.testing.testbrowser
* Leave backwards compatibility imports in place in
zope.testbrowser.testing
I'd be willing to have a look at this if Benji were OK in principle on
this and someone was willing to review it.
--
Brian Sutherland
___
Zope-Dev maillist - Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
https://mail.zope.org/mailman/listinfo/zope-announce
https://mail.zope.org/mailman/listinfo/zope )
