Guido van Rossum guido@digicool.com writes:
Michael Hudson wrote:
The use of Vladimir Marangoz's obmalloc patch in some of the benchmarks sparked a discussion about whether this patch should be incorporated into Python 2.1. There was support from many for adding it on an opt-in basis, since when nothing has happened...
... I'm still waiting on BDFL pronouncement on this one. The plan was to check it in for beta1 on an opt-in basis (Vladimir has written the patch this way).
-- Marc-Andre Lemburg
If it is truly opt-in (supposedly a configure option?), I'm all for it.
It is very much opt-in.
I recall vaguely though that Jeremy or Tim thought that the patch touches lots of code even when one doesn't opt in. That was a no-no so close before the a2 release. Anybody who actually looked at the code got an opinion on that now?
I suggest looking at the patch. Not at the code, but what it does as a diff:
1) Add a file Objects/obmalloc.c 2) Add stuff to configure.in & config.h to detect the --with-pymalloc argument to ./configure 3) Conditionally #include "obmalloc.h" in Objects/object.c if WITH_PYMALLOC is #defined 4) Conditionally #define the variables in Include/objimpl.h to #define the #defines needed to override the memory imiplementation if WITH_PYMALLOC is #defined
And *that's it*. That's not my definition of "touches a lot of code".
Cheers, M.
If it is truly opt-in (supposedly a configure option?), I'm all for it.
It is very much opt-in.
I recall vaguely though that Jeremy or Tim thought that the patch touches lots of code even when one doesn't opt in. That was a no-no so close before the a2 release. Anybody who actually looked at the code got an opinion on that now?
I suggest looking at the patch. Not at the code, but what it does as a diff:
- Add a file Objects/obmalloc.c
- Add stuff to configure.in & config.h to detect the --with-pymalloc argument to ./configure
- Conditionally #include "obmalloc.h" in Objects/object.c if WITH_PYMALLOC is #defined
- Conditionally #define the variables in Include/objimpl.h to #define the #defines needed to override the memory imiplementation if WITH_PYMALLOC is #defined
And *that's it*. That's not my definition of "touches a lot of code".
OK, I just looked, and I agree. BTW, for those who want to look, the URL is:
http://sourceforge.net/patch/?func=detailpatch&patch_id=101104&group...
This is currently assigned to Barry. Barry, can you see if this is truly fit for inclusion? Or am I missing something?
Note that there's a companion patch that adds a memory profiler:
http://sourceforge.net/patch/?func=detailpatch&patch_id=101229&group...
Should this also be applied? Is there a reason why it shouldn't?
--Guido van Rossum (home page: http://www.python.org/~guido/)
"GvR" == Guido van Rossum guido@digicool.com writes:
GvR> This is currently assigned to Barry. Barry, can you see if GvR> this is truly fit for inclusion? Or am I missing something?
I think I was wary of applying it without the chance to run it through Insure when it was enabled. I can put that on my list of things to do for beta1.
-Barry
"Barry A. Warsaw" wrote:
"GvR" == Guido van Rossum guido@digicool.com writes:
GvR> This is currently assigned to Barry. Barry, can you see if GvR> this is truly fit for inclusion? Or am I missing something?
I think I was wary of applying it without the chance to run it through Insure when it was enabled. I can put that on my list of things to do for beta1.
That's a good idea, but why should it stop you from checking the patch in ? After all, it's opt-in, so people using it will know that they are building non-standard stuff. Perhaps we ought to add a note '(experimental)' to the configure flags ?!
Guido van Rossum wrote:
If it is truly opt-in (supposedly a configure option?), I'm all for it.
It is very much opt-in.
I recall vaguely though that Jeremy or Tim thought that the patch touches lots of code even when one doesn't opt in. That was a no-no so close before the a2 release. Anybody who actually looked at the code got an opinion on that now?
I suggest looking at the patch. Not at the code, but what it does as a diff:
- Add a file Objects/obmalloc.c
- Add stuff to configure.in & config.h to detect the --with-pymalloc argument to ./configure
- Conditionally #include "obmalloc.h" in Objects/object.c if WITH_PYMALLOC is #defined
- Conditionally #define the variables in Include/objimpl.h to #define the #defines needed to override the memory imiplementation if WITH_PYMALLOC is #defined
And *that's it*. That's not my definition of "touches a lot of code".
OK, I just looked, and I agree. BTW, for those who want to look, the URL is:
http://sourceforge.net/patch/?func=detailpatch&patch_id=101104&group...
This is currently assigned to Barry. Barry, can you see if this is truly fit for inclusion? Or am I missing something?
Note that there's a companion patch that adds a memory profiler:
http://sourceforge.net/patch/?func=detailpatch&patch_id=101229&group...
Should this also be applied? Is there a reason why it shouldn't?
Since both patches must be explicitely enabled by a configure switch I'd suggest to apply both of them -- this will give them much more testing. In the long run, I think that using such an allocator is better than trying maintain free lists for each type seperatly.