xlat performance / "compile time" parsing?
I recently made some changes to our FreeRADIUS logging config, inspired by Matthew Newton's creative use of linelog as discussed on the -users list. The config uses a wrapping unlang policy, along with quite a bit of conditional expansion. Since deploying it, I've noticed the CPU use of our FreeRADIUS server has jumped, at lot. Hotspot profiling with "oprofile" tells me that almost all the time is being spent inside a few critical functions: radius_xlat rad_copy_variable radius_evaluate_condition I've stared at the code for the first two a bit, and I think I understand why it's so expensive; it essentially parses the input string one character at a time, every time, and in the case of a conditional attribute, it walks over the input twice (or more). A roughly similar thing happens wth radius_evaluate_condition. This set me to wondering about some kind of parser cache. The idea would be that a string like: "var %{%{foo}:-%{%{bar}:-baz}}" ...would be parsed into some kind of linked-list / tree, like so: v[0].type = LITERAL v[0].value = "var " v[1].type = CONDITIONAL v[1].value = struct { left.type = EXPANSION left.value = "foo" right.type = CONDITIONAL right.value = struct { left.type = EXPANSION left.value = "bar" right.type = LITERAL right.value = "baz" } } The parsing would be done transparently by radius_xlat the first time it saw an input template, and cached for the lifetime of the process - though I haven't yet figured out how to do this without locking. Once parsed, the expansion function should be both very simple, and very fast. If we wanted to go even further, the EXPANSION values could be the attribute number rather than name, but I am having trouble reasoning out whether this might lead to problems with changing xlat or attribute registrations e.g. on HUP or similar. Something similar could be done for radius_evaluate_condition, except it's even easier there - only the unlang code calls uses conditions, so we can just compile it - no need to transparently convert the string. Before I embark on this to see if it helps - is this even a reasonable approach? Cheers, Phil
Phil Mayers wrote:
I've stared at the code for the first two a bit, and I think I understand why it's so expensive; it essentially parses the input string one character at a time, every time, and in the case of a conditional attribute, it walks over the input twice (or more).
Well, that's an obvious place to fix things.
A roughly similar thing happens wth radius_evaluate_condition.
This set me to wondering about some kind of parser cache. The idea would be that a string like:
"var %{%{foo}:-%{%{bar}:-baz}}"
This could maybe be done for "unlang". For other expansions, it's a lot more difficult. My first priority would be to simplify the parsing of conditional expansions. If that helps, it's likely good enough.
...would be parsed into some kind of linked-list / tree, like so:
Like a language...
Before I embark on this to see if it helps - is this even a reasonable approach?
Maybe. I'm scared of any complex solution. I'd much rather find out what's wrong with radius_xlat, and go fix it. Alan DeKok.
participants (2)
-
Alan DeKok -
Phil Mayers