xlat performance / "compile time" parsing?

[Phil Mayers]

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