Minor but fun changes today
After some discussion, I moved the REQUESTs to using a talloc pool. i.e. it allocates a 32K chunk of memory for reach request, and then allocates REQUEST, RADIUS_PACKET, VALUE_PAIR, etc. within that. If the pool runs out of memory, talloc() will fall back to using malloc. So there’s no downside to this. We’ve made a few other minor changes, too. Preliminary performance tests show a 35% decrease in time (clock cycles, via valgrind) to process the default configuration, with one user: bob Cleartext-Password := “bob” I don’t expect the server to get 35% faster for everything. Once it hits a database or uses EAP-TLS, all bets are off. Those things are *slow*. But the changes today were ~600 LoC, for a 35% decrease in processing time. That’s not bad. Alan DeKok.
On Tue, Feb 10, 2015 at 05:05:23PM -0500, Alan DeKok wrote:
After some discussion, I moved the REQUESTs to using a talloc pool. i.e. it allocates a 32K chunk of memory for reach request, and then allocates REQUEST, RADIUS_PACKET, VALUE_PAIR, etc. within that. If the pool runs out of memory, talloc() will fall back to using malloc. So there’s no downside to this. We’ve made a few other minor changes, too.
That makes sense - they're all hung off from each other anyway, so it should make cleaning up much easier as you don't need to worry about cleaning up each individual allocated block. My first computer had 32k memory...
But the changes today were ~600 LoC, for a 35% decrease in processing time. That’s not bad.
Nice. I thought talloc was generally slower (because of the management overhead). I guess the efficiencies come from a single malloc, rather than malloc/free all over the place when required. In my experience, malloc can be painfully slow. Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Specialist, Infrastructure Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
On Feb 10, 2015, at 5:41 PM, Matthew Newton <mcn4@leicester.ac.uk> wrote:
That makes sense - they're all hung off from each other anyway, so it should make cleaning up much easier as you don't need to worry about cleaning up each individual allocated block.
Exactly.
My first computer had 32k memory…
Mine had 4K. :( I spent a fair bit of time optimizing an embedded OS for an 8 bit CPU. Lots of fun… the same principles apply here. Don’t make it faster, make it do less.
I thought talloc was generally slower (because of the management overhead). I guess the efficiencies come from a single malloc, rather than malloc/free all over the place when required. In my experience, malloc can be painfully slow.
Yes. The gain from using pools is MUCH more than the loss from the talloc() overhead. Alan DeKok.
On 10/02/15 22:05, Alan DeKok wrote:
After some discussion, I moved the REQUESTs to using a talloc pool. i.e. it allocates a 32K chunk of memory for reach request, and then allocates REQUEST, RADIUS_PACKET, VALUE_PAIR, etc. within that. If the pool runs out of memory, talloc() will fall back to using malloc. So there’s no downside to this. We’ve made a few other minor changes, too.
Preliminary performance tests show a 35% decrease in time (clock cycles, via valgrind) to process the default configuration, with one user:
Cool. I was going to suggest a talloc_pool() for REQUEST's a while back but never got round to doing any measurement. I guess the pool size could be a global but it's almost certainly unnecessary - most requests won't be around for long enough for it to matter. Glad to hear it does make a difference - nice going!
On Feb 11, 2015, at 10:35 AM, Phil Mayers <p.mayers@imperial.ac.uk> wrote:
Cool. I was going to suggest a talloc_pool() for REQUEST's a while back but never got round to doing any measurement.
It’s noticeably faster. While we were on that work, we made a few other changes which improved other areas.
I guess the pool size could be a global but it's almost certainly unnecessary - most requests won't be around for long enough for it to matter.
The talloc documentation says to *not* use a pool per request. But their use-case is a bit different, I think. It works, it’s faster, and the code changes are small. Alan DeKok.
On 11 Feb 2015, at 10:35, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 10/02/15 22:05, Alan DeKok wrote:
After some discussion, I moved the REQUESTs to using a talloc pool. i.e. it allocates a 32K chunk of memory for reach request, and then allocates REQUEST, RADIUS_PACKET, VALUE_PAIR, etc. within that. If the pool runs out of memory, talloc() will fall back to using malloc. So there’s no downside to this. We’ve made a few other minor changes, too.
Preliminary performance tests show a 35% decrease in time (clock cycles, via valgrind) to process the default configuration, with one user:
Cool. I was going to suggest a talloc_pool() for REQUEST's a while back but never got round to doing any measurement.
I guess the pool size could be a global but it's almost certainly unnecessary - most requests won't be around for long enough for it to matter.
It'd be a bit of a pain to keep track TBH. You can use a single pool because of thread safety issues, so you'd need to keep track of the number of requests allocated. What would be great is if talloc tracked high/low water marks, in terms of pool memory actually allocated. That way the server could automatically adjust allocations based on the stats generated when the request was freed.
Glad to hear it does make a difference - nice going!
In a real world test with a moderately complex config, authenticating against OpenLDAP 2.4 we measured about a 5.2% increase in overall performance, which is actually very good for 6 hours of Alan's time :) This is the low hanging fruit though, for bigger gains re-architecting is likely required. -Arran Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS development team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On 11 Feb 2015, at 10:58, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
On 11 Feb 2015, at 10:35, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 10/02/15 22:05, Alan DeKok wrote:
After some discussion, I moved the REQUESTs to using a talloc pool. i.e. it allocates a 32K chunk of memory for reach request, and then allocates REQUEST, RADIUS_PACKET, VALUE_PAIR, etc. within that. If the pool runs out of memory, talloc() will fall back to using malloc. So there’s no downside to this. We’ve made a few other minor changes, too.
Preliminary performance tests show a 35% decrease in time (clock cycles, via valgrind) to process the default configuration, with one user:
Cool. I was going to suggest a talloc_pool() for REQUEST's a while back but never got round to doing any measurement.
I guess the pool size could be a global but it's almost certainly unnecessary - most requests won't be around for long enough for it to matter.
It'd be a bit of a pain to keep track TBH. You can use a single pool because of thread safety issues, so you'd need to keep track of the number of requests allocated.
*can't use. As you already know, as you were the one who discovered that particular issue :) Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS development team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On 11/02/15 15:58, Arran Cudbard-Bell wrote:
It'd be a bit of a pain to keep track TBH. You can use a single pool because of thread safety issues, so you'd need to keep track of the number of requests allocated.
Sorry, not what I meant - rather the figure "32kb" for each REQUEST. Some sites might allocate a lot of crap in their configs, others little, so it might be something to vary. But it's probably pointless optimisation!
On 11 Feb 2015, at 11:08, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 11/02/15 15:58, Arran Cudbard-Bell wrote:
It'd be a bit of a pain to keep track TBH. You can use a single pool because of thread safety issues, so you'd need to keep track of the number of requests allocated.
Sorry, not what I meant - rather the figure "32kb" for each REQUEST. Some sites might allocate a lot of crap in their configs, others little, so it might be something to vary.
But it's probably pointless optimisation!
It's already configurable (shhh, don't tell anyone). But it's one of those knobs that really shouldn't exist, and should be determined automatically by the server. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS development team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On Wed, Feb 11, 2015 at 11:15:28AM -0500, Arran Cudbard-Bell wrote:
On 11 Feb 2015, at 11:08, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
Sorry, not what I meant - rather the figure "32kb" for each REQUEST. Some sites might allocate a lot of crap in their configs, others little, so it might be something to vary.
But it's probably pointless optimisation!
It's already configurable (shhh, don't tell anyone). But it's one of those knobs that really shouldn't exist, and should be determined automatically by the server.
Is there any reason why the server can't keep track of the average size of the last <n> requests, and dynamically adjust the amount of memory it allocates for the request pool? Not sure if there is a way in, say, talloc free to find out how much memory was actually freed in total? Then it would be an easy job at free time to keep track of how much was used for that request, and feed back into a loop to determine the allocation size. But maybe it's just overcomplicating things. Worst case you don't want the situation that most requests go 10 octets over 32k - so some debug or similar to tell the admin this has happened would be useful. Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Specialist, Infrastructure Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
On 11 Feb 2015, at 12:32, Matthew Newton <mcn4@LEICESTER.AC.UK> wrote:
On Wed, Feb 11, 2015 at 11:15:28AM -0500, Arran Cudbard-Bell wrote:
On 11 Feb 2015, at 11:08, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
Sorry, not what I meant - rather the figure "32kb" for each REQUEST. Some sites might allocate a lot of crap in their configs, others little, so it might be something to vary.
But it's probably pointless optimisation!
It's already configurable (shhh, don't tell anyone). But it's one of those knobs that really shouldn't exist, and should be determined automatically by the server.
Is there any reason why the server can't keep track of the average size of the last <n> requests, and dynamically adjust the amount of memory it allocates for the request pool?
That's what I was suggesting (in a previous email). But the talloc API doesn't track or expose such information currently.
Not sure if there is a way in, say, talloc free to find out how much memory was actually freed in total? Then it would be an easy job at free time to keep track of how much was used for that request, and feed back into a loop to determine the allocation size.
Right but there's significant churn in the context of the request as it runs through all the different modules. i.e. one module could allocate a 1M chunk in the context, then free it, and at cleanup time you'd never know that had happened. Well you hope you wouldn't know that had happened. To use contexts efficiently you have to do try and stick to LAFF (Last Allocated First Freed). That's not always possible, and in those cases the context will leak memory (in a controlled way). i.e. if you allocate chunk A, then chunk B, then free chunk A, the memory isn't returned to the context. The talloc code ain't got the smarts to deal with memory fragmentation, or doesn't want the overhead. Calling talloc_free_children() on a pool, will reset it back to its initial state. So we could in theory maintain a pool of pools, and avoid all runtime mallocs(). The issue is whether the overhead of managing the pool of pools is greater than the overhead of calling malloc.
But maybe it's just overcomplicating things. Worst case you don't want the situation that most requests go 10 octets over 32k - so some debug or similar to tell the admin this has happened would be useful.
The only way to do that currently would be to set a memlimit on the context, which, when exceeded would cause allocations to fail, and the server to exit. Not ideal. -Arran Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS development team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On Feb 11, 2015, at 12:32 PM, Matthew Newton <mcn4@leicester.ac.uk> wrote:
Is there any reason why the server can't keep track of the average size of the last <n> requests, and dynamically adjust the amount of memory it allocates for the request pool?
It’s harder. My $0.02 would be to do a debug build which has that instrumentation. But I don’t think it should be there in a production environment. It’s 2015. Just spend another $25 and put more RAM into the system. FreeRADIUS already uses near-zero RAM. So having it use a little bit more isn’t a problem.
But maybe it's just overcomplicating things. Worst case you don't want the situation that most requests go 10 octets over 32k - so some debug or similar to tell the admin this has happened would be useful.
It’s a configuration option which will confuse most people, TBH. Just set it to a reasonably large value, and ignore it. The *worst* things that will happen are: a) too large pool: the server will use a little more memory than before b) too small pool: the performance will go down a little bit over a larger pool size Alan DeKok.
On Wed, Feb 11, 2015 at 03:52:14PM -0500, Alan DeKok wrote:
It’s 2015. Just spend another $25 and put more RAM into the system. FreeRADIUS already uses near-zero RAM. So having it use a little bit more isn’t a problem. ... It’s a configuration option which will confuse most people, TBH. Just set it to a reasonably large value, and ignore it. The *worst* things that will happen are:
Yup, you're right. Make it a hidden option as now, and for 99% of people it won't ever matter and performance will be better than now. For the odd case it's needed, the option is there. Cheers, Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Specialist, Infrastructure Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
participants (4)
-
Alan DeKok -
Arran Cudbard-Bell -
Matthew Newton -
Phil Mayers