Hello,
working to integrate the PV module I reached the AVPs and after a short analyze I wonder (and ask you) whether you should drop the naming scheme we have in Kamailio (integer id and string name) and stick to string names all the time. ser seems to use mostly that although looks like support for ID is still there, but I cannot see how they can be used with the naming scheme in config (e.g., is it possible to have $f.i:34 in config to refer to avp with id 34?).
The benefits I see: - complexity in dealing with integer id or string name - avp name aliases removed
Note that even for string-named AVPs there is an integer id computed and used in comparisons to speed up.
Jan in an email some time ago mentioned that no performance penalty could be observed. I pasted most of the content of Jan's email about ser avps at: http://sip-router.org/wiki/devel/avps-ser
Looking at the code where there are lot of IF name is string or integer, I wonder if really worth the complexity. Today we have lot of other variables for quick and shared-memory operations ($var(...), $shv(...), $sht(...), $dbr(...) and latest $mct(...)), avps not being anymore used for everywhere.
Drawback is the naming scheme backward compatibility. We can treat "i:20" simply as string or throw error at startup (I prefer the second because is cleaner in long time).
Opinions?
Cheers, Daniel
On Thursday 12 March 2009, Daniel-Constantin Mierla wrote:
working to integrate the PV module I reached the AVPs and after a short analyze I wonder (and ask you) whether you should drop the naming scheme we have in Kamailio (integer id and string name) and stick to string names all the time. ser seems to use mostly that although looks like support for ID is still there, but I cannot see how they can be used with the naming scheme in config (e.g., is it possible to have $f.i:34 in config to refer to avp with id 34?).
Hi Daniel,
i think its a good idea to go only for string names. We use only AVP aliases all the time in our configurations, because this typed names were so cumbersome to remember and type.
Drawback is the naming scheme backward compatibility. We can treat "i:20" simply as string or throw error at startup (I prefer the second because is cleaner in long time).
I also think the second approach is better.
Cheers,
Henning
Daniel-Constantin Mierla writes:
working to integrate the PV module I reached the AVPs and after a short analyze I wonder (and ask you) whether you should drop the naming scheme we have in Kamailio (integer id and string name) and stick to string names all the time.
i'm using exclusively integer ids and would oppose dropping them. this kind of radical change simply cannot be done unless you can do it such a way that integer names are internally auto-converted to strings. this change would affect not only the script, but also db tables where all my avp name columns are ints.
i once looked at kamailio code and got the feeling that lookup with integer names was faster than with string names and therefore started to use integer names.
The benefits I see:
- complexity in dealing with integer id or string name
- avp name aliases removed
i have never used name aliases.
-- juha
one way might be to consider "i:30" as a string that starts with letter "i".
-- juha
On 03/12/2009 10:06 PM, Juha Heinanen wrote:
one way might be to consider "i:30" as a string that starts with letter "i".
My only concern is about making the code a hell by trying to keep some backward compatibility with old configs. In the past we went via radical changes, like with break replaced by exit or return.
I would prefer a strict config checker that will report errors rather than some stupid mistakes done because inside c code one adds an avp with name "34" an in script is still "i:34" and it is expected to be the same, but because now both are considered strings they are different.
To rephrase, I prefer either to drop completely the integer id or stay with current. Trying to automatically convert may bring other traps at runtime.
However, interger id will be compatible only with Kamailio PV model, ser avp naming scheme does not use them anymore in config language (some ser devel can correct me here if i got it wrong).
Cheers, Daniel
Daniel-Constantin Mierla writes:
On 03/12/2009 10:06 PM, Juha Heinanen wrote:
one way might be to consider "i:30" as a string that starts with letter "i".
I would prefer a strict config checker that will report errors rather than some stupid mistakes done because inside c code one adds an avp with name "34" an in script is still "i:34" and it is expected to be the same, but because now both are considered strings they are different.
i didn't propose that. avp with name "i:34" would be different avp than one with name "34" or "s:34". all three would just be different string valued avps.
in radius response the name is prefixed with # if it is int, e.g., #34. the code that loads the avps would then install this avp with string name "i:34".
-- juha
On 03/12/2009 09:59 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
working to integrate the PV module I reached the AVPs and after a short analyze I wonder (and ask you) whether you should drop the naming scheme we have in Kamailio (integer id and string name) and stick to string names all the time.
i'm using exclusively integer ids and would oppose dropping them. this kind of radical change simply cannot be done unless you can do it such a way that integer names are internally auto-converted to strings. this change would affect not only the script, but also db tables where all my avp name columns are ints.
the modules that load the AVPs should take care of keeping only string names inside. However, s: and i: are only indications of the name type in config file. Inside the code these are marked by flags.
I do not know how you load the avps, but in database (usr_preferences table), the avp name column is string. The type is indicating whether to convert to int or keep it as string.
i once looked at kamailio code and got the feeling that lookup with integer names was faster than with string names and therefore started to use integer names.
Did you measure how faster is? Here some details about the avp name and matching algorithm:
- if the avp has id name, then this is kept on a 16b field and nothing else - if the avp has string name, then a hash id is computed and kept in the 16b and the name in a str
When matching ids, then the 16b fields are compared. When matching strings, then the 16b fields are compared, if equal, then the length of the names are compared and if equal, the content of names are compared.
At the end it is about code complexity vs. some potential performance.
The benefits I see:
- complexity in dealing with integer id or string name
- avp name aliases removed
i have never used name aliases.
not using them as well, I'm using M4.
Cheers, Daniel
Daniel-Constantin Mierla writes:
not using them as well, I'm using M4.
i too have been using M4 for avp names. the advantage over string names is that if i mistype a M4 macro AVP name, parsing of kamailio.cfg fails. if i mistype string avp name, it goes unnoticed and can lead to difficult to find bug.
so perhaps there should be a possibility to declare avps so that undeclared would make parsing to fail.
-- juha
Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
not using them as well, I'm using M4.
i too have been using M4 for avp names. the advantage over string names is that if i mistype a M4 macro AVP name, parsing of kamailio.cfg fails. if i mistype string avp name, it goes unnoticed and can lead to difficult to find bug.
so perhaps there should be a possibility to declare avps so that undeclared would make parsing to fail.
I think that's a reasonable thing in general. I'm wondering how we still could deal with dynamnically produced AVPs (i.e., those produced by loading from user profile). I think as long as these are not referred from the script, it is okay, otherwise they have to be declared. Modules exchanging info via AVPs don't care about declaration.
-jiri
-- juha
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
-
Daniel-Constantin Mierla -
Henning Westerholt -
jh＠tutpro.com -
Jiri Kuthan