1

By profiling (with ProDelphi profiler) my app I found that I could achieve a better performance if this function would be faster:

function BooleanToYN(isTrue: Boolean): string;
begin
  if isTrue then
    Result := 'Y'
  else
    Result := 'N';
end;

As i saw this code I thought it is already "optimized". Is theree a way to speed it up in your opinion? Any other comment? This is legacy code, I didn't write it, so don't ask me why it is like this. Of course I can consider to change how the app is written, anyway if there is a way to speed this simple function up (may be using another ready made Delphi function) it would be great.

UnDiUdin
  • 14,924
  • 39
  • 151
  • 249
  • Are you sure you need to make it faster or do you simply need to find a way to call it fewer times? Remember: A profiler can only help you find hot spots, but can't tell you what their cause is. – afrazier Oct 14 '13 at 15:22
  • You're not saying what percent that function is responsible for, or if it is exclusive or inclusive time, or if you were sampling CPU or wall-clock time. It might be your "bottleneck" and maybe not. As afrazier says, you might very well be calling it much more often than necessary. I don't use profilers - I use [*this method*](http://stackoverflow.com/a/18984752/23771). (Check out the first comment, from David H.) You need to get the big picture. All profilers do it give you a scattering of confusing measurements. – Mike Dunlavey Oct 14 '13 at 20:50

2 Answers2

9

Your profiler is most likely not giving you reliable information. It is simply implausible that this function is your program's bottleneck.

Your main options for improving that function are:

  1. To inline the function.
  2. To remove the branch.

You would implement option 2 like this:

function BooleanToYN(isTrue: Boolean): string;
const
  BoolStr: array [Boolean] of string = ('N', 'Y');
begin
  Result := BoolStr[isTrue];
end;

Of course, it would make good sense to make the function inline. In other words, apply both of my options above.

But it would be astounding if that made any discernible difference to your program's performance. Think about it for just a moment. Is your program really spending a significant proportion of its runtime executing that function? Really?

Profiling is very difficult and you should expect profilers to give mis-information. When using a profiler you should operate with an underlying level of distrust. Always be initially sceptical of its output.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
6

I had a theory that Char would be faster than String, but sometimes you just need to try it....

It can be improved 16x by returning a CHAR instead of String. I expect that it's got to do with the allocation of the string.

I dropped the original code into a DUnit test framework and added a test to run it in a loop 100 million times. I called it YNSLoop (S for string), then made another called YNCLoop (C for Char) which returns Char instead of String, then I made another based on David's code with the Array, and I called that YNALoop (A for Array).
Then I made an inlined version of the original, per David's suggestion. That was good, so I made an Inlined Char version as well. And hey, the inline version of the Char version is even faster.

YNSLoop:      4487 ms  (Original)
YNSILoop:     1226 ms  (Original, Inlined)
YNCLoop:       266 ms  (Char instead of String)
YNCILoop:      124 ms  (Char Inlined)
YNALoop:      4548 ms  (Array)

Conclusion: If you can get away using Char instead of String, do it. Either way, Inline it if you can.

Code below:

function BooleanToYNSI(isTrue: Boolean): string;     inline;
begin
  if isTrue then
    Result := 'Y'
  else
    Result := 'N';
end;

function BooleanToYNS(isTrue: Boolean): string;
begin
  if isTrue then
    Result := 'Y'
  else
    Result := 'N';
end;

function BooleanToYNC(isTrue: Boolean): Char;
begin
  if isTrue then
    Result := 'Y'
  else
    Result := 'N';
end;

function BooleanToYNCI(isTrue: Boolean): Char;      inline;
begin
  if isTrue then
    Result := 'Y'
  else
    Result := 'N';
end;

function BooleanToYNArray(isTrue: Boolean): string;
const
  BoolStr: array [Boolean] of string = ('N', 'Y');
begin
  Result := BoolStr[isTrue];
end;

procedure TDBISAM_PERFTest.YNSLoop;
var
i : integer;
begin
  for i := 1 to 100000000 do
    if BooleanToYNS(True) <> 'Y' then
      Fail('Failed');
end;

procedure TDBISAM_PERFTest.YNSILoop;
var
i : integer;
begin
  for i := 1 to 100000000 do
    if BooleanToYNSI(True) <> 'Y' then
      Fail('Failed');
end;

procedure TDBISAM_PERFTest.YNCLoop;
var
i : integer;
begin
  for i := 1 to 100000000 do
    if BooleanToYNC(True) <> 'Y' then
      Fail('Failed');
end;

procedure TDBISAM_PERFTest.YNCILoop;
var
i : integer;
begin
  for i := 1 to 100000000 do
    if BooleanToYNCI(True) <> 'Y' then
      Fail('Failed');
end;

procedure TDBISAM_PERFTest.YNALoop;
var
i : integer;
begin
  for i := 1 to 100000000 do
    if BooleanToYNArray(True) <> 'Y' then
      Fail('Failed');
end;
Chris Thornton
  • 15,620
  • 5
  • 37
  • 62
  • What about Davids comment on my comment about the extra heap allocation? I'm confused now. – Pieter B Oct 14 '13 at 14:24
  • 3
    I think your test should have been: `var s: string;` and then in the loop `s := BooleanToWhatEver`; – kobik Oct 14 '13 at 14:28
  • 2
    Your benchmark is flawed. At the call site, in the real program, the caller must put this function into the context of a string. So there will be a conversion to string and hence a heap allocation. So you must make sure that your code replicates that. It's also kind of stupid to optimise a function that is not the bottleneck. – David Heffernan Oct 14 '13 at 14:29
  • 1
    @DavidHeffernan, you are making assumptions about the usage of the function. It may, or may not, be needed as a string. If it's called like this: if BooleanToYN(SomeBoolExpression) = 'Y' then ... Then the conversion is not needed. Hence my adivice: " If you can get away using Char instead of String..." – Chris Thornton Oct 14 '13 at 15:12
  • @ChrisThornton If that's the usage then you would write `if SomeBoolExpression then` Do you honestly believe that this function is the bottleneck? – David Heffernan Oct 14 '13 at 15:13
  • 1
    @DavidHeffernan, no I have made no such claim. It's only a bottleneck if you run it a million times in a loop, so the evidence pretty much demonstrates that it is NOT the bottleneck, unless the program itself is as trivial as my DUnit 100-million-loop test. I was only curious to see the impact of changing a function returns a single-character string, would benefit from returning a char instead of the string. I also wanted to see how much faster inlining would make it (sure did), and whether or not the array trick would yield anything. I went through the exercise, and shared the results. – Chris Thornton Oct 14 '13 at 15:30
  • You should publish the complete program so that we can run it. You should also include versions that put the value returned from the Char functions into a string. – David Heffernan Oct 14 '13 at 15:39
  • Anyway, my main point here is that I cannot really imagine what anyone would do with a `char` that has one of two values based on the value of a `Boolean`. In my opinion, if ever you found yourself in that scenario, you would simply store the `Boolean`. And that can be seen by the contraction of the use case that you suggested in your first comment. You'd only ever use code like that in the Q when putting the value into a string. Now, you might write `s[1] := BooleanToYN(...)` I suppose. But even then I simply cannot think of a scenario where returning a `char` would result in performance gain – David Heffernan Oct 14 '13 at 15:52