classification
Title: Remove special block allocation from floatobject.c
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: kristjan.jonsson, mark.dickinson, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2012-03-29 09:54 by kristjan.jonsson, last changed 2012-03-31 13:10 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
nofreelist.patch kristjan.jonsson, 2012-03-29 09:54 review
nofreelist.patch kristjan.jonsson, 2012-03-29 23:36 review
Messages (11)
msg157016 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-29 09:54
floatobject.c has its own block allocator.  This appears to be ancient, from before the time when obmalloc.c was invented.
This patch removes this allocator and puts an upper limit on the freelist of floats.  The purpose of this is to reduce memory fragmentation, since blocks used for floats cannot be used for anything else in python.  These blocks tend to stay around for a long time, it is sufficient for one float from each of the 1k blocks to be alive to keep that block present in memory forever.

It is the presence of the fast freelist that is the performance enhancer here, not the fast block allocator.
msg157060 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-29 16:10
One thing: PyFloat_ClearFreeList() is supposed to return the number of objects previously in the freelist, not zero.
Also, perhaps 10 is a bit on the small side for the number of objects kept on the freelist. 100 instead? Or do you think that's too large?
msg157071 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-29 17:07
Yes, it is supposed to, but no one is actually looking at that value.  It was used in debugging information during PyFloat_Fini() which is no longer relevant if this block information is removed.

Sure, 100 or 10 does not matter, I see 100 being used in some other freelists.

Also, I wasn't able to see any negative benefits of this using pybench.

Here's a thought:  I think using the linked list approach using Py_TYPE() is rather neat.  Other freelists are mostly implemented using static arrays of pointers.  Any thoughts on this?  We could unify the approach taken.
msg157072 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-29 17:09
> Yes, it is supposed to, but no one is actually looking at that value.
> It was used in debugging information during PyFloat_Fini() which is no
> longer relevant if this block information is removed.

Still, let's honour the API rather than break it.

> Here's a thought:  I think using the linked list approach using
> Py_TYPE() is rather neat.  Other freelists are mostly implemented
> using static arrays of pointers.  Any thoughts on this?  We could
> unify the approach taken.

I don't have any preference either way, but I see little point in
unifying the approach, since no common code is shared.
msg157073 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-29 17:16
Correction:  The number returned was the number of floats in existence, not the size of the freelist.  Do you think I should add a counter to support that functionality?  I´d rather change it to be the size of the old freelist, similar to PyTuple_ClearFreeList().
msg157074 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-29 17:20
> Do you think I should add a counter to support that functionality?  I
> ´d rather change it to be the size of the old freelist, similar to
> PyTuple_ClearFreeList().

It should be the size of the old freelist, indeed.
msg157114 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-29 23:36
Here is a new patch, with the suggested changes.  The variable names and macros are similarly named as those for other objects such as lists.
msg157128 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-30 07:50
> Here is a new patch, with the suggested changes.  The variable names
> and macros are similarly named as those for other objects such as
> lists.

Looks good to me.
msg157131 - (view) Author: Roundup Robot (python-dev) Date: 2012-03-30 09:19
New changeset 37ebe64d39d2 by Kristján Valur Jónsson in branch 'default':
Issue #14435: Remove special block allocation code from floatobject.c
http://hg.python.org/cpython/rev/37ebe64d39d2
msg157132 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-30 09:21
All done.
msg157186 - (view) Author: Roundup Robot (python-dev) Date: 2012-03-31 13:10
New changeset 10fcaf5903e6 by Kristján Valur Jónsson in branch 'default':
Issue #14435: Add Misc/NEWS and Misc/ACKS
http://hg.python.org/cpython/rev/10fcaf5903e6
History
Date User Action Args
2012-03-31 13:10:20python-devsetmessages: + msg157186
2012-03-30 09:21:08kristjan.jonssonsetstatus: open -> closed
resolution: fixed
messages: + msg157132
2012-03-30 09:19:54python-devsetnosy: + python-dev
messages: + msg157131
2012-03-30 07:50:28pitrousetmessages: + msg157128
2012-03-30 06:58:32mark.dickinsonsetnosy: + mark.dickinson
2012-03-29 23:36:26kristjan.jonssonsetfiles: + nofreelist.patch

messages: + msg157114
2012-03-29 17:20:34pitrousetmessages: + msg157074
2012-03-29 17:16:39kristjan.jonssonsetmessages: + msg157073
2012-03-29 17:09:54pitrousetmessages: + msg157072
2012-03-29 17:07:22kristjan.jonssonsetmessages: + msg157071
2012-03-29 16:10:28pitrousetnosy: + pitrou
messages: + msg157060
2012-03-29 09:54:09kristjan.jonssoncreate