Re: [Zope-dev] Zope 2.6 planning - call for contributors!
seb bacon wrote [CallProfiler] FWIW, my own opinion is that it should not take the 'MonkeyPatch' approach.
Why? Any other approach means a slowdown in the Zope code regardless of whether profiling is turned on or off... monkeypatching means you end up with zero slowdown when not profiling. Anthony -- Anthony Baxter <anthony@interlink.com.au> It's never to late to have a happy childhood.
--- Anthony Baxter <anthony@ekit-inc.com> wrote:
seb bacon wrote [CallProfiler] FWIW, my own opinion is that it should not take the 'MonkeyPatch' approach.
Why? Any other approach means a slowdown in the Zope code regardless of whether profiling is turned on or off... monkeypatching means you end up with zero slowdown when not profiling.
Anthony
I agree, monkey patches are perfect for this. That makes them totally transparent to the application and Zope for that matter. There's nothing wrong with them in the right application. -Casey __________________________________________________ Do You Yahoo!? Yahoo! Sports - sign up for Fantasy Baseball http://sports.yahoo.com
On Mon, 4 Mar 2002 14:40, Casey Duncan wrote:
I agree, monkey patches are perfect for this. That makes them totally transparent to the application and Zope for that matter. There's nothing wrong with them in the right application.
My main concern is the use of monkeypatching in the core makes it difficult for someone else to release a product that also MPs without them worrying about whether something has already patched code. Especially when we're talking about MP'ing so many core Zope objects (yes, I count >1 as "so many" :) I think the performance hit is really quite minimal for two if statements at the entry and exit point(s) of a function to turn the behaviour on and off. Richard ps. don't forget Anthony, our first reaction when we both thought of this approach was *shudder* :)
On Mon, 2002-03-04 at 03:47, Richard Jones wrote:
On Mon, 4 Mar 2002 14:40, Casey Duncan wrote:
I agree, monkey patches are perfect for this. That makes them totally transparent to the application and Zope for that matter. There's nothing wrong with them in the right application.
My main concern is the use of monkeypatching in the core makes it difficult for someone else to release a product that also MPs without them worrying about whether something has already patched code. Especially when we're talking about MP'ing so many core Zope objects (yes, I count >1 as "so many" :)
I agree - altering classes at runtime is less predictable or discoverable than defining them statically. I think the current solution is really nice, but I don't see that it has any particular benefits over a static implementation, which has the benefit of following a standard, well-known pattern.
I think the performance hit is really quite minimal for two if statements at the entry and exit point(s) of a function to turn the behaviour on and off.
Yes - I would bet the performace difference is in the order of hundredths of a second. seb
seb bacon wrote:
Yes - I would bet the performace difference is in the order of hundredths of a second.
Which I would prefer not to have added to the several hundred other hundredths-of-a-second little differences-that-people-thought-wouldn't-make-a-difference that have been added to Zope over time... cheers, Chris
On Mon, 2002-03-04 at 10:47, Chris Withers wrote:
seb bacon wrote:
Yes - I would bet the performace difference is in the order of hundredths of a second.
Which I would prefer not to have added to the several hundred other hundredths-of-a-second little differences-that-people-thought-wouldn't-make-a-difference that have been added to Zope over time...
What, like ZPT? ;-P http://zope.nipltd.com/public/lists/dev-archive.nsf/ByKey/4084B02199CC6AFB (to save the bother of following the link, that's the thread from about a month ago regarding evidence suggesting ZPT may be *twice* as slow as DTML) seb
seb bacon wrote:
http://zope.nipltd.com/public/lists/dev-archive.nsf/ByKey/4084B02199CC6AFB
(to save the bother of following the link, that's the thread from about a month ago regarding evidence suggesting ZPT may be *twice* as slow as DTML)
Yup. And I'm a pretty strong advocate that ZPT should be speeded up a lot. Anyone got any idea who I ened to help to do this? cheers, Chris
On Mon, 2002-03-04 at 11:23, Chris Withers wrote:
seb bacon wrote:
http://zope.nipltd.com/public/lists/dev-archive.nsf/ByKey/4084B02199CC6AFB
(to save the bother of following the link, that's the thread from about a month ago regarding evidence suggesting ZPT may be *twice* as slow as DTML)
Yup. And I'm a pretty strong advocate that ZPT should be speeded up a lot.
Indeed. However, I was being a bit glib with my example, and didn't explain my point properly: that performance issues should be subordinate to good design. Therefore, I suspect MonkeyPatching is bad: Pros - a tiny performance gain Cons - unpredictable interaction with future products; not a well-known method of distributing products; not easily discoverable But perhaps my 'cons' are misplaced? Mostly, I'm uneasy that someone looking at ZPublisher code would have no way of knowing that CallProfiler hooks into it if it were monkeypatched. seb seb
seb bacon wrote:
Pros - a tiny performance gain Cons - unpredictable interaction with future products;
I'd rephrase that as a pro: Ability to work with products of the future, without the need to understand their inner workings...
not a well-known method of distributing products;
Urm... that I gotta disagree with... Tehre are now many such products for Zope...
not easily discoverable
What do you mean by discoverable?
But perhaps my 'cons' are misplaced? Mostly, I'm uneasy that someone looking at ZPublisher code would have no way of knowing that CallProfiler hooks into it if it were monkeypatched.
Yeah they would, 'cos CallProfiler will be well documented beforeit hits a release and will onyl come into play if explicity enabled. And if you explicitly enable it, you should read the documentation first... cheers, Chris
I have to say I agree, up to a point. I think that monkeypatching goes right to the very heart of the language - Python was written not just to allow it, but it's opperation almost encorages it (Sort of). HOWEVER, I am somewhat against a monkeypatch being made part of the core distribution (No disrespect to the authors of such products - me included), these should be optional downloads, up to the point that the code is rolled into the core codebase. The call profiler seems to be a very popular feature, and it seems to provide very useful information. It's not rock solid yet (I am sure I even remember it's author saying this a day or so ago), but it will be, and that is the point that it should be considered for rolling in - Possibly to replace the existing debug/profiling stuff, that seems to be somewhat overshadowed by it... Just my 2c (Or 0.02 E, take you pick) Adrian... -- The difficulty of tactical maneuvering consists in turning the devious into the direct, and misfortune into gain. - Sun Tzu ----- Original Message ----- From: "Chris Withers" <chrisw@nipltd.com> To: "seb bacon" <seb@jamkit.com> Cc: "Richard Jones" <rjones@ekit-inc.com>; "Casey Duncan" <casey_duncan@yahoo.com>; "Anthony Baxter" <anthony@ekit-inc.com>; <zope-dev@zope.org> Sent: Monday, March 04, 2002 12:54 PM Subject: [Zope-dev] Re: MonkeyPatching in the Core (was: Zope 2.6 planning)
seb bacon wrote:
Pros - a tiny performance gain Cons - unpredictable interaction with future products;
I'd rephrase that as a pro:
Ability to work with products of the future, without the need to understand their inner workings...
not a well-known method of distributing products;
Urm... that I gotta disagree with... Tehre are now many such products for Zope...
not easily discoverable
What do you mean by discoverable?
But perhaps my 'cons' are misplaced? Mostly, I'm uneasy that someone looking at ZPublisher code would have no way of knowing that CallProfiler hooks into it if it were monkeypatched.
Yeah they would, 'cos CallProfiler will be well documented beforeit hits a release and will onyl come into play if explicity enabled. And if you explicitly enable it, you should read the documentation first...
cheers,
Chris
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
On Tue, 5 Mar 2002 10:54, Adrian Hungate wrote:
The call profiler seems to be a very popular feature, and it seems to provide very useful information. It's not rock solid yet (I am sure I even remember it's author saying this a day or so ago)
Any idea what I might have been referring to? I don't recall having said anything about the profiler in any forums except these about any topic other than integrating it into the core. And certainly nothing about stability. Mind you, I was quite ill late last week, and I might have unwittingly sent some email off in my delerious state... :) Richard
Sorry, I think this was a mental aberration, as I have been completely unable to find the email I thought I saw in the archives... I must have been thinking of something else, and someone else, and for the life of me I can not think who, or what... Sorry. Adrian... -- The difficulty of tactical maneuvering consists in turning the devious into the direct, and misfortune into gain. - Sun Tzu ----- Original Message ----- From: "Richard Jones" <rjones@ekit-inc.com> To: <zope-dev@zope.org> Cc: "Adrian Hungate" <adrian@haqa.co.uk> Sent: Tuesday, March 05, 2002 12:04 AM Subject: [Zope-dev] CallProfiler (was: MonkeyPatching in the Core (was: Zope 2.6 planning))
On Tue, 5 Mar 2002 10:54, Adrian Hungate wrote:
The call profiler seems to be a very popular feature, and it seems to provide very useful information. It's not rock solid yet (I am sure I even remember it's author saying this a day or so ago)
Any idea what I might have been referring to? I don't recall having said anything about the profiler in any forums except these about any topic other than integrating it into the core. And certainly nothing about stability.
Mind you, I was quite ill late last week, and I might have unwittingly sent some email off in my delerious state... :)
Richard
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
What about something like apple's Extension Manager, where you could disable/enable 'sets' of products? Though frankly, it's not too tough to just move the subdirectory somewhere and restart... but it would be a way to have a TTW way of configuring your Zope, and having the option of 'loading up' the distribution with optional stuff (which might be disabled by default?)
From: "Adrian Hungate" <adrian@haqa.co.uk>
I think that monkeypatching goes right to the very heart of the language - Python was written not just to allow it, but it's opperation almost encorages it (Sort of). HOWEVER, I am somewhat against a monkeypatch being made part of the core distribution (No disrespect to the authors of such products - me included), these should be optional downloads, up to the point that the code is rolled into the core codebase.
Just as a follow up to my last comment: I think it would be a very good idea to have a common framework with the basic distribution for patches (Who cam up with monkeypatch?? nasty name), and to that end I offer PatchKit as a starting point. The aim in my sugestion is to allow ideas such as Marc's here to be applied to patches as well has fully formed products. If all patches were loaded into a PatchManager, there could be a global off switch, and an individual off switch for each patch. This would also encorage patch writers to make their patches in a common style, that could be easily queried (On the Control_Panel or a sub page) to report on patches and their basic actions. I could go on, but I think you can see where I am going here... (Sort of components I guess) :) Adrian... -- The difficulty of tactical maneuvering consists in turning the devious into the direct, and misfortune into gain. - Sun Tzu ----- Original Message ----- From: "marc lindahl" <marc@bowery.com> To: "Adrian Hungate" <adrian@haqa.co.uk>; "Chris Withers" <chrisw@nipltd.com>; "seb bacon" <seb@jamkit.com> Cc: "Richard Jones" <rjones@ekit-inc.com>; "Casey Duncan" <casey_duncan@yahoo.com>; "Anthony Baxter" <anthony@ekit-inc.com>; <zope-dev@zope.org> Sent: Tuesday, March 05, 2002 2:48 AM Subject: Re: [Zope-dev] Re: MonkeyPatching in the Core (was: Zope 2.6planning)
What about something like apple's Extension Manager, where you could disable/enable 'sets' of products? Though frankly, it's not too tough to just move the subdirectory somewhere and restart... but it would be a way to have a TTW way of configuring your Zope, and having the option of 'loading up' the distribution with optional stuff (which might be disabled by default?)
From: "Adrian Hungate" <adrian@haqa.co.uk>
I think that monkeypatching goes right to the very heart of the language - Python was written not just to allow it, but it's opperation almost encorages it (Sort of). HOWEVER, I am somewhat against a monkeypatch being made part of the core distribution (No disrespect to the authors of such products - me included), these should be optional downloads, up to the point that the code is rolled into the core codebase.
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
Indeed. However, I was being a bit glib with my example, and didn't explain my point properly: that performance issues should be subordinate to good design. Therefore, I suspect MonkeyPatching is bad:
Pros - a tiny performance gain Cons - unpredictable interaction with future products; not a well-known method of distributing products; not easily discoverable
But perhaps my 'cons' are misplaced? Mostly, I'm uneasy that someone looking at ZPublisher code would have no way of knowing that CallProfiler hooks into it if it were monkeypatched.
Monkey-patching should be thought of as a last resort, when it absolutely-positively-has-to-get-there-overnight (security hotfixes, etc.), for exactly the cons described above. Speaking as one who has been bitten by this sort of thing before (with the bite marks to prove it!), I can attest that the only thing worse than confusing code is _invisible_ code. That said -- I think that Richard's approach is quite powerful, and I think that there is a middle ground here. I think that the call profiler could be brought into the core with minimal changes and without being "invisible" or seeming to promote monkey-patching. What if, instead of the static list of callable info that the CP currently uses, Zope objects could register themselves as profilable? We would then make sure that the object types that CP handles now register themselves, but other products that we don't know (or have to know) about could register themselves too if they wanted. Think of this as "consentual" monkey-patching (hmm... may have to change this metaphor soon!). The products have to take some explicit action to be profilable, so it is not invisible in the code of the product. The hooks will continue to installed as-needed, so there is no performance issue. Thoughts? Brian Lloyd brian@zope.com Software Engineer 540.361.1716 Zope Corporation http://www.zope.com
What if, instead of the static list of callable info that the CP currently uses, Zope objects could register themselves as profilable? We would then make sure that the object types that CP handles now register themselves, but other products that we don't know (or have to know) about could register themselves too if they wanted.
Think of this as "consentual" monkey-patching (hmm... may have to change this metaphor soon!). The products have to take some explicit action to be profilable, so it is not invisible in the code of the product. The hooks will continue to installed as-needed, so there is no performance issue.
Thoughts?
Bingo! seb
With respect to the overhead of inserting things into modules in the source code, Fred Drake (I think!) pointed out to me that constructs in the form if __debug__: ... are automatically removed by the parser when Python is run with the -O flag. Note that I am *not* sure how Zope performs when run with Python -O. However, assuming it does run OK, then it makes sense to put all sorts of conditional things into code which are disabled for performance when -O is passed on the command line to Python. Also note that I guarantee Zope will not run with Python -OO, as this removes doc strings, which the Publisher relies upon. At the time, I was pushing more for a slightly more complex form of run time diagnostic control without the overhead of continuous symbol lookup, e.g. ifcondition(bitmask): such that either all the bits in the bitmask were "enabled" in the current debugging context, or at least some of them were. Then again, I effectively want exits in Python's ceval.c at various locations to be able to do "magic" without the burden of more interpretation. For instance, there are actually some clever things you can do with the new C level profiler, but 'stealing' the profiler to do other magic is not really the right approach. -- Matt Kromer Zope Corporation http://www.zope.com/
On Tue, 5 Mar 2002 07:20, Brian Lloyd wrote:
Seb wrote:
Pros - a tiny performance gain Cons - unpredictable interaction with future products; not a well-known method of distributing products; not easily discoverable
What if, instead of the static list of callable info that the CP currently uses, Zope objects could register themselves as profilable? We would then make sure that the object types that CP handles now register themselves, but other products that we don't know (or have to know) about could register themselves too if they wanted.
This doesn't really address the concern I have with regard to the con given above. The main reason is that the call profiler's monkeypatching is done potentially many times, and it performs an unpatch as well as a patch. This muckery can potentially really stuff over other components that also monkeypatch the same methods. Especially if they _also_ perform unpatching. I'm not sure the consentual monkeypatch approach really solves this issue... and if the product has to be modified to make it register itself with the profiler, why not just have it include the (very simple) calls to the profiler instead? Richard
On Mon, 4 Mar 2002 15:20:58 -0500, "Brian Lloyd" <brian@zope.com> wrote:
I can attest that the only thing worse than confusing code is _invisible_ code.
The main page of the Control_Panel already displays a fairly long list of name/value pairs describing the state of the zope installation: Zope Version, INSTANCE_HOME, etc. Maybe Products should be able to include 'headline news' which also gets displayed there. ? Toby Dickenson tdickenson@geminidataloggers.com
Not a bad idea. On that note, can we add an API to add Control_Panel icons please? Adrian... -- The difficulty of tactical maneuvering consists in turning the devious into the direct, and misfortune into gain. - Sun Tzu ----- Original Message ----- From: "Toby Dickenson" <tdickenson@devmail.geminidataloggers.co.uk> To: "Brian Lloyd" <brian@zope.com> Cc: "seb bacon" <seb@jamkit.com>; "Chris Withers" <chrisw@nipltd.com>; "Richard Jones" <rjones@ekit-inc.com>; "Casey Duncan" <casey_duncan@yahoo.com>; "Anthony Baxter" <anthony@ekit-inc.com>; <zope-dev@zope.org> Sent: Tuesday, March 05, 2002 11:00 AM Subject: Re: [Zope-dev] MonkeyPatching in the Core (was: Zope 2.6 planning) On Mon, 4 Mar 2002 15:20:58 -0500, "Brian Lloyd" <brian@zope.com> wrote:
I can attest that the only thing worse than confusing code is _invisible_ code.
The main page of the Control_Panel already displays a fairly long list of name/value pairs describing the state of the zope installation: Zope Version, INSTANCE_HOME, etc. Maybe Products should be able to include 'headline news' which also gets displayed there. ? Toby Dickenson tdickenson@geminidataloggers.com _______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
Anthony Baxter wrote:
seb bacon wrote [CallProfiler] FWIW, my own opinion is that it should not take the 'MonkeyPatch' approach.
Why? Any other approach means a slowdown in the Zope code regardless of whether profiling is turned on or off... monkeypatching means you end up with zero slowdown when not profiling.
...and in this case, I wouldn't call it MonkeyPatch-ing, just taking advantage of Python's extremely dynamic nature for a very good cause. cheers, Chris
participants (10)
-
Adrian Hungate -
Anthony Baxter -
Brian Lloyd -
Casey Duncan -
Chris Withers -
marc lindahl -
Matthew T. Kromer -
Richard Jones -
seb bacon -
Toby Dickenson