Question

I'm working on an Ada based project that I'm not terribly familiar with, and I've just seen something that at first glance seems inefficient, but of course that all depends on what the compiler might do.

if Ada.Strings.Fixed.Trim
    (Source => Test_String,
     Side   => Ada.Strings.Both) =
    String_1 then
    --Do something here
elsif Ada.Strings.Fixed.Trim
    (Source => Test_String,
     Side   => Ada.Strings.Both) =
    String_2 then

    --Do something else here
end if;

I feel that it would be more efficient to call the Trim procedure and store the result in a String variable, then test against different Strings in each condition of the if statement, especially if there are many conditions to check (never mind that using a binary search might be even better). Of course, I may be wrong, so my question is, is there something about compile time optimisation in Ada that I do not know about, that might cause the Trim function to only be called once, and only have the result tested in each condition of the if statement?

Was it helpful?

Solution 2

I don't think we need to worry about efficiency for this case. But the coding style that do need to be improved. To define a variable to hold the result of the Trim function is a better practice in my opinion. The program logic is clearer and easier for maintenance.

Let's say you want to change the Trim function to another one or change the parameter passing in, you only need to change one place. Although for this case there are only two places, you still have a chance to error. If there are more cases to test, then there definitely will be missing case.

OTHER TIPS

That would be compiler-dependent, not language-dependent. Certainly, GNAT GPL 2013 calls Trim twice both at -O2 and at -O3.

Your (and my) intuition seems to be right: do the trim once and store the result ...

Trimmed : constant String :=
  Ada.Strings.Fixed.Trim (Source => Test_String, Side => Ada.Strings.Both);

... though personally I’d write

Trimmed : constant String :=
  Ada.Strings.Fixed.Trim (Test_String, Side => Ada.Strings.Both);

on the grounds that in this case no one should need the named parameter association to clarify the programmer’s intention!

While I have no disagreement whatsoever with your assessment or Simon's answer, it's worth bearing in mind what optimisation actually means...

If each call to "trim" takes (conservatively) 0.1ms of CPU time, and the rewrite takes 2 minutes, the breakeven point in time saved is over 1 million executions of that particular statement!

This of course ignores (a) on the positive side, the value of gaining experience, and (b) on the negative side, the fact that CPU time is nowadays less valuable than your time. And (c) the time we have spent discussing the optimisation too!

With respect to "compile time optimisation in Ada" , the Ada language doesn't say anything about this. All it would say here is that the program must behave as if the function were called twice, i.e. it must produce the same results. But if the compiler "knows" that the function would produce the exact same result the second time, it can generate code that calls it only once. It can't do this for a function call in general, because a function could include side effects or use global variables that whose values could have changed. In this case, since the effects of Ada.Strings.Fixed.Trim are defined by the language and since those effects do guarantee that the effects would be the same, a compiler could, in theory, mark this function as one for which a call with the exact same parameters could be optimized. (A function in a Pure package would definitely be one where a second call could be eliminated, but unfortunately Ada.Strings.Fixed isn't defined by the Ada language as being Pure.) To find out whether a compiler actually does this particular optimization, though, you'd have to try it and check the code. I believe that if Test_String is not marked as Volatile, then compilers are allowed to assume that its value will be the same for each Ada.Strings.Fixed.Trim call (i.e. it can't be changed by another task in between the calls), but I'm not 100% sure about this.

I'd declare a constant to hold the result (like Simon), but for me this is more out of a desire to avoid duplicated code than it is out of a concern for efficiency.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top