Improve performance of external (UDR) functions

[asfernandes]

Test preparation:

recreate function div_udr (
    n1 integer,
    n2 integer
) returns double precision
    external name 'udf_compat!UC_div'
    engine udr;

create domain d_int_check integer check (value > 0);
create domain d_double_check double precision check (value >= 0);

recreate function div_udr2 (
    n1 d_int_check,
    n2 d_int_check
) returns d_double_check
    external name 'udf_compat!UC_div'
    engine udr;

Comparison:

set stats on;

-- 4.4s -> 2.7s
execute block
as
    declare n integer = 5000000;
    declare x integer;
begin
    while (n > 0) do
    begin
        n = n - 1;
        x = div_udr(3, 2);
    end
end;

-- 5.1s -> 3.2s
execute block
as
    declare n integer = 5000000;
    declare x integer;
begin
    while (n > 0) do
    begin
        n = n - 1;
        x = div_udr2(3, 2);
    end
end;

[sim1984]

Does this only improve the performance of external functions only, or is there an effect for procedures/triggers too?

[asfernandes]

Does this only improve the performance of external functions only, or is there an effect for procedures/triggers too?

It's mostly about functions only.

There could be a marginal improvement with procedures with parameters with constraints, but I didn't tested it, as procedure/trigger implementation could also be greater improved, but is much more difficult.

[asfernandes]

@dyemanov are you going to review this before merge?

[dyemanov]

In some places we still have the code like this:

if (DSC_EQUIV(src, dst))
  memcpy
else
  MOV_move

Do you think it could improve something? I know we also have the same trick inside CVT_move_common(), but it's two calls deeper in the stack...

[asfernandes]

It's already being done in CVT_move_common.
It's not inline, but, I think this is not going to be so relevant.

[dyemanov]

Maybe add a named constant e.g. MSG_NULL_FLAG = -1 instead of using magic numbers directly in code?

[asfernandes]

I think FB_FALSE / FB_TRUE could be used as we used them extensively in similar places.

[dyemanov]

Being paranoid, I'd also add an assertion about fmt_count being even ;-)

[asfernandes]

It should be wrong. It's used to initialize output message, that is going to have the null flag.
In this case, it purposely ignores it.

[dyemanov]

I see nothing this scope would protect. Is it really needed?

[asfernandes]

I used it mostly as a block to not create a separate function just for it.
I'm not removing the explicit scope label.

[dyemanov]

IIRC, this condition/assignment is redundant, EVL_expr() is guaranteed to return nullptr in this case.