Commented by: Sean Leyne (seanleyne)

Since the case is about "slower" performance, some mention of the performance of non-collation based query vs. collation query would be helpful.

Commented by: Toni Martir (toni_martir)

I'm sorry this is not a issue about performance for non-collation based or collation based, the performance with collation or non-collation is near the same. As I included in the original information COLLATE UNICODE_CI is as fast as not using COLLATE. This is an issue about UNICODE_CI_AI only (or maybe relative to any _AI collation), that is about Accent Insensitive collation.

Commented by: Toni Martir (toni_martir)

I debugged the firebird server, and found the bottleneck. I know nothing about unicode implementation or icu library, but it seems the way Firebird is calling the library is slowing things a lot.
I have found a workaround but because my knowledgment is very limited someone else should validate (or fix the bug in other way).

The bottleneck is in unicode_util.cpp near line 1253 (2.5 branch):
UTransliterator *trans = icu->utransOpen("Any-Upper; NFD; [:Nonspacing Mark:] Remove; NFC",
UTRANS_FORWARD, NULL, 0, NULL, &errorCode);

It seems that utransOpen is somewhat slow, so to solve the problem this "handle" to transliterator should be created only once in some way.

Adding this lines to create the transliterator only once speed up things:
// Add variable to store the transliterator
UTransliterator* globaltrans = NULL;
.....
if (globaltrans == NULL)
globaltrans = icu->utransOpen("Any-Upper; NFD; [:Nonspacing Mark:] Remove; NFC",
UTRANS_FORWARD, NULL, 0, NULL, &errorCode);
UTransliterator *trans = globaltrans;

and remove the line closing the transliterator icu->utransClose(trans) and the handle should be closed on cleanup.

After this fix, the UNICODE_CI and UNICODE_CI_AI have near the same performance (a query that longs for 10 seconds now takes only 300 milliseconds)

Commented by: @asfernandes

Thanks Toni for debug it.

A transliteration could not be shared between threads, so I used to clone it, which seems to not degrade the performance.

Commented by: Toni Martir (toni_martir)

Thanks for the fast fix.
Just to comment that In C#⁠ exists the [ThreadStatic] attribute so one variable per thread is initialized, I can remember something in C like "static _thread", but don't know if it's ok for all C compilers. Anyway I usually try to avoid this variables when multitheading by using an object instance for each thread to store thread object references, I don't know how this "problems" are solved in Firebird code.

Thanks again.

Commented by: @asfernandes

This is not so simple. It would require a "TLS cleanup" routine too, to not leak memory of destroyed threads.

We currently does not have this in place in the code, so the clone approach seems doable and faster enough.

Commented by: Toni Martir (toni_martir)

I'm sorry, I compiled and debugged with the updated unicode_util.cpp and tested the sample I provide, and now UNICODE_CI_AI does not work correctly, it returns all the rows, so it's now broken.
With the changes I proposed works ok (but it's not the good fix because it's not thread safe).
I also measured the penalty for using clone, and it's about 50% time for comparisons than using UNICODE_CI, using Open only once has no penalty, just the same time UNICODE_CI and UNICODE_CI_AI.
Note: A 50% may be acceptable (if it works).

Thanks

Commented by: @asfernandes

The problem happens with the embedded ICU 3.0, but not with newer versions. I'm investigating why.

Commented by: @asfernandes

I committed a different fix. Please test it.

Commented by: Toni Martir (toni_martir)

Now works ok and witthout any performance penalty.
Thanks