Jump to content
Maxime Janvier

Avoid parameter evaluation

Recommended Posts

Hi,


I have quite a tricky problem, and can't figure out a proper solution.
You see, I want to log debug strings in my application, so I wrote a module where the only entry point is a pure method "procedure Log(const LogLevel: TLogLevel; const Str: String);". 
So the goal would be to only log strings if the logger is set to log at least that log level. That way, I can easily change the logger to only log error strings for example. 


So far so good.


Problem is, when I write things like "Log(llTrace, 'Test ' + GetLastError + BuildStringFromContext); for example, the string will be evaluated wether I log it or not, because the log filter is done inside the function.
I was wandering if there was a way to avoid the evaluation of the string when I know I won't log it?
Of course the obvious solution would be to write 
"if CanLog(llTrace) then Log('Test ' + GetLastError + BuildStringFromContext);"
But that would be tedious to write, and make the code harder to read.


So I though of two possibilities:
- First: Using inline function, to replace at compile time the first Log line by the if/then statement. Unfortunately, even with inline function, the arguments are fully eveluated when the function is called (tested). So it's a dead end.
- I also though to use a preprocessor macro, to replace at compile time the fist line by the if/then statement, but Delphi doesn't support C/C++ like defines, it can only set/evaluate booleans.

 

So do you think of a solution i could've overlooked?


Without any solution, I'll stick to my first Log version, as I value code cleanliness over performance, but it's still a bit frustrating knowing my code will execute instructions for nothing at all if I disable my log....
Regards,

Share this post


Link to post

May be something like this, but it looks even more complicated then the test with if CanLog... 😄

 

type tLogFunction = reference to function: string;

procedure Log(aLevel: integer; aGetLogText: tLogFunction);
 var s: string;
 begin
 if aLevel>1 then begin
  s:=aGetLogText;
  writeln(s);
 end;
end;

...

Log(0, function:string begin
 result:='abc ' + IntToStr(random(100));
end);

Log(2, function:string begin
 result:='efg ' + IntToStr(random(100));
end);

 

Share this post


Link to post

That would be a solution indeed.

But performance-wise, I don't know the cost of anonymous functions.. What if that's called in a loop?

Even if it' not called, some memory has to be dynamically allocated for the function code + the captured variables + the cost of capturing those variables...

I might do a benchmark for that...

Share this post


Link to post

There is few ways i can think of like these 2 approaches

type
  TLogLevel = (llInfo, llTrace, llError);

procedure Log(LogLevel: TLogLevel; const LogText: string); overload;
begin

end;

procedure Log(LogLevel: TLogLevel; const Text: array of string); overload;
var
  Count: Integer;
begin
  Count := Length(Text);
  case LogLevel of
    llInfo:  ;
    llTrace:
      begin
        if Count > 0 then
        begin

        end;
      end;
    llError:   ;
  end;
end;

procedure Log(LogLevel: TLogLevel; const LogText: string = ''; const Error: string = ''; const Additional: string = ''); overload;
begin

end;

procedure Log(LogLevel: TLogLevel; const LogText: string = ''; ErrorCode: Integer = 0; const Additional: string = ''); overload;
begin

end;

procedure TForm10.FormCreate(Sender: TObject);
var
  BuildStringFromContext: string;
begin
  Log(llTrace, ['Test', IntToStr(GetLastError), BuildStringFromContext]);

  Log(llError, 'Test', GetLastError, BuildStringFromContext);

  Log(llError, 'Simple Text', 0);
  Log(llInfo, 'Simple Text', 0, 'Additional Info');
  Log(llInfo, 'Simple Text', '');
end;

Just food for thoughts for you, using array of string is easy and will make you life easier, but you should remember the order, while the overloaded versions with default empty values, are more readable and harder to confuse.

 

Each had its own shortcoming, like i had to add 0 do the ambiguity, but if you make you mind you can build them right and remove the need for these extra 0 or ''.

It is up to your need to think of a solution that you are comfortable with.

Share this post


Link to post
4 minutes ago, Maxime Janvier said:

That would be a solution indeed.

But performance-wise, I don't know the cost of anonymous functions.. What if that's called in a loop?

Even if it' not called, some memory has to be dynamically allocated for the function code + the captured variables + the cost of capturing those variables...

I might do a benchmark for that...

In a loop, I would store the result of CanLog to a local boolean variable and then use "if StoredValueOfCanLog>=..." and direct call to your Log procedure (without anonymous function).

Share this post


Link to post
28 minutes ago, Kas Ob. said:

There is few ways i can think of like these 2 approaches


type
  TLogLevel = (llInfo, llTrace, llError);

procedure Log(LogLevel: TLogLevel; const LogText: string); overload;
begin

end;

procedure Log(LogLevel: TLogLevel; const Text: array of string); overload;
var
  Count: Integer;
begin
  Count := Length(Text);
  case LogLevel of
    llInfo:  ;
    llTrace:
      begin
        if Count > 0 then
        begin

        end;
      end;
    llError:   ;
  end;
end;

procedure Log(LogLevel: TLogLevel; const LogText: string = ''; const Error: string = ''; const Additional: string = ''); overload;
begin

end;

procedure Log(LogLevel: TLogLevel; const LogText: string = ''; ErrorCode: Integer = 0; const Additional: string = ''); overload;
begin

end;

procedure TForm10.FormCreate(Sender: TObject);
var
  BuildStringFromContext: string;
begin
  Log(llTrace, ['Test', IntToStr(GetLastError), BuildStringFromContext]);

  Log(llError, 'Test', GetLastError, BuildStringFromContext);

  Log(llError, 'Simple Text', 0);
  Log(llInfo, 'Simple Text', 0, 'Additional Info');
  Log(llInfo, 'Simple Text', '');
end;

Just food for thoughts for you, using array of string is easy and will make you life easier, but you should remember the order, while the overloaded versions with default empty values, are more readable and harder to confuse.

 

Each had its own shortcoming, like i had to add 0 do the ambiguity, but if you make you mind you can build them right and remove the need for these extra 0 or ''.

It is up to your need to think of a solution that you are comfortable with.

Thanks for these approaches!

Unfortunately, the strings (or the functions that'll generate the strings) will still be individually evaluated, if I call Log(ttTrace, [SysErrorMessage(GetLastError)]);, the SysErrorMessage function will still be called.

Share this post


Link to post
29 minutes ago, Vandrovnik said:

In a loop, I would store the result of CanLog to a local boolean variable and then use "if StoredValueOfCanLog>=..." and direct call to your Log procedure (without anonymous function).

That's actually good!

I think that I'll normally use the Log(LogLevel, MyString), but when obvious optimizations appear, I could add a CanLog to avoid evals in loops...

 

In that way, I can have a proper balance between code cleanliness/ease of reading and performances.

Share this post


Link to post
14 minutes ago, Maxime Janvier said:

if I call Log(ttTrace, [SysErrorMessage(GetLastError)]);, the SysErrorMessage function will still be called.

Well my point was about to not call SysErrorMessage in the first place, just pass the GetLastError, even more on this, if the code between Log and where actually it implemented can't modify The GetLastError, then you could use ttTraceIWithError and there you can call SysErrorMessage.

In case you care about performance then just use a method and track the generated assembly

image.thumb.png.f3a8767e280012982b117e2316e74b24.png

 

And by replacing the most repeated or even the specific phrases like 'Test' and 'Exception'... with LOG_String_XX you can short and fast logging.

 

Again food for thoughts.

Share this post


Link to post
5 minutes ago, Kas Ob. said:

Well my point was about to not call SysErrorMessage in the first place, just pass the GetLastError, even more on this, if the code between Log and where actually it implemented can't modify The GetLastError, then you could use ttTraceIWithError and there you can call SysErrorMessage.

In case you care about performance then just use a method and track the generated assembly

image.thumb.png.f3a8767e280012982b117e2316e74b24.png

 

And by replacing the most repeated or even the specific phrases like 'Test' and 'Exception'... with LOG_String_XX you can short and fast logging.

 

Again food for thoughts.

Good points!

 

That would require a bit of work, but might worth it!

 

Thank you very much for your time and thoughts, much appreciated! :)

Share this post


Link to post
8 hours ago, Maxime Janvier said:

- I also though to use a preprocessor macro, to replace at compile time the fist line by the if/then statement, but Delphi doesn't support C/C++ like defines, it can only set/evaluate booleans.

FreePascal supports preprocessor macros; https://www.freepascal.org/docs-html/3.2.0/prog/progse5.html

 

For Delphi, you would have to write/find an external tool to preprocess your code before compiling it.

Share this post


Link to post

Having the logging expressions compiled and executed on a regular basis means they are less likely to break unnoticed and cause issues when you need them. 

 

Building logging strings is usually a small fraction of execution time so keeping things simple is a valid choice. 

Share this post


Link to post
11 hours ago, Kas Ob. said:

procedure Log(LogLevel: TLogLevel; const Text: array of string); overload;

 

Since performance is in question, I would suggest taking this approach a step further and use 'array of const' instead of 'array of string'.  That way, you can pass in integers and other fundamental types as-is and not invoke the overhead of converting them to strings unless you are sure you actually need to, eg:

procedure Log(LogLevel: TLogLevel; const Args: array of const); overload;
begin
  if CanLog(LogLevel) then
  begin
    // convert Args to a log message and write it out as needed...
  end;
end;

Log(llTrace, ['Test', GetLastError(), ...]);

 

  • Like 3

Share this post


Link to post
9 hours ago, Remy Lebeau said:

use 'array of const' instead of 'array of string'. 

I never thought about this.

 

Thank you !

Share this post


Link to post

Can't shake it off without trying to validate the type, so here what i tried and it is working perfectly

{$APPTYPE CONSOLE}

{$R *.res}

uses
  System.SysUtils;

procedure DoProcess(const Args: array of const);
var
  i, Count: Integer;
begin
  Count := Length(Args);
  if Count > 0 then
    for i := 0 to Pred(Count) do
    begin
      case Integer(Args[i].VType) of
        vtString, vtChar, vtWideChar, vtAnsiString, vtWideString, vtUnicodeString:
          Writeln(string(Args[i].VString));

        vtInteger:
          Writeln(Args[i].VInteger);

        vtInt64:
          Writeln(Int64(Args[i].VInt64));
      else
        Writeln('Type handling not implemented yet ' + IntToStr(Args[i].VType));
      end;
    end;
end;

procedure Test;
var
  st: string;
begin
  st := 'Param1';
  DoProcess([st, 'Param2', 5]);
end;

begin
  Test;
  Readln;
end.

output

Quote

Param1
Param2
5

 

The generated assembly is nice and short, one thing though, don't forget to declare Args itself as const as in procedure DoProcess(const Args: array of const) or it will be copied locally, in other words it will be ugly.

Share this post


Link to post

I cannot believe how complex this discussion has become!  All to avoid having a tiny unit with four boolean variables defined in the interface, along with a GetLogLevel function which can be called when needed, and then writing, for example:

if LOGINFO then log(MyComplexStringExpression);

if LOGALL then Log(AnotherExpression);

This seems to meet all the criteria of readability, simplicity and efficiency.

Share this post


Link to post
2 hours ago, timfrost said:

This seems to meet all the criteria of readability, simplicity and efficiency.

It's cumbersome to have that if statement all the time.

Share this post


Link to post
48 minutes ago, dummzeuch said:

It's cumbersome to have that if statement all the time.

If log level is defined in compile time, he could also use $IFDEF or $IF instead 😄 

Share this post


Link to post
Posted (edited)
8 hours ago, Kas Ob. said:

Can't shake it off without trying to validate the type, so here what i tried and it is working perfectly

Not quite.  You have implemented DoProcess() incorrectly, causing undefined behavior.  The code compiles, but it will not behave correctly at runtime for certain types.  For instance, try passing a single Char into it and your code will crash.

8 hours ago, Kas Ob. said:

      case Integer(Args[i].VType) of
        vtString, vtChar, vtWideChar, vtAnsiString, vtWideString, vtUnicodeString:
          Writeln(string(Args[i].VString));

 

That approach is wrong.  You cannot treat all of those types the same way, as they are stored in different ways in the array.  vtChar and ctWideChar are stored as individual values in the array itself (like integers are), whereas vtString is a pointer to a ShortString variable, whereas vtAnsiString and vtUnicodeString are copies of the data block pointer from inside of an AnsiString/UnicodeString instance, etc.

 

You need to handle the various types individually and correctly, eg:

procedure DoProcess(const Args: array of const);
var
  i: Integer;
begin
  for i := Low(Args) to High(Args) do
  begin
    case Args[i].VType of
      vtString:
        Writeln(Args[i].VString^);

      vtChar:
        Writeln(Args[i].VChar);

      vtPChar:
        Writeln(Args[i].VPChar);

      vtWideChar:
        Writeln(Args[i].VWideChar);

      vtPWideChar:
        Writeln(Args[i].VPWideChar);

      vtAnsiString:
        Writeln(AnsiString(Args[i].VAnsiString));

      vtWideString:
        Writeln(WideString(Args[i].VWideString));

      vtUnicodeString:
        Writeln(UnicodeString(Args[i].VUnicodeString));

      vtInteger:
        Writeln(Args[i].VInteger);

      vtInt64:
        Writeln(Int64(Args[i].VInt64));
    else
      Writeln('Type handling not implemented yet ' + IntToStr(Args[i].VType));
    end;
  end;
end;

Also, see the MakeStr() example in this earlier post:

 

Edited by Remy Lebeau
  • Like 1
  • Thanks 1

Share this post


Link to post
4 hours ago, Vandrovnik said:

If log level is defined in compile time, he could also use $IFDEF or $IF instead 😄 

Yes, some special characters plus if plus ifend are a lot more convenient than an if then statement. I'm sold.

  • Haha 2

Share this post


Link to post
On 7/26/2024 at 4:08 AM, timfrost said:

I cannot believe how complex this discussion has become!  All to avoid having a tiny unit with four boolean variables defined in the interface, along with a GetLogLevel function which can be called when needed ...

Delphi is a few decades behind contemporary language features today. The last several language additions have been around since the 90's. I'm not sure how far into the 2000's they are yet. I think I'll be long dead before they catch up with the more popular language features that most languages already support today in 2024.

 

I mean ... hey ... the only thing added in D12 was multi-line string literals. WOOT! WOOT!

 

I'm not moving off of D10.4.2 until they start focusing on real language improvements. Given how many emails I'm getting lately trying to urge people to get back onto maintenance plans, I'm guessing the user base is shrinking ... maybe because nobody wants to use a language that's still stuck in the 90's when most contemporary languages are still evolving quite steadily.

 

I think Delphi introduced anonymous methods and closures before Java did. But since then, Java has updated them with nearly every release, and they look nothing like they did initially. I can't think of anything that has changed with Delphi's syntax.

  • Sad 1

Share this post


Link to post
44 minutes ago, David Schwartz said:

I mean ... hey ... the only thing added in D12 was multi-line string literals. WOOT! WOOT!

do not forget that multi-line strings completely broke the IDE. For D13 they will have to change the code in the IDE to support them.

Share this post


Link to post
1 hour ago, David Schwartz said:

I think Delphi introduced anonymous methods and closures before Java did.

No, you are wrong. Even anonymous methods were in Java first.

Share this post


Link to post
Posted (edited)
3 hours ago, dummzeuch said:

No, you are wrong. Even anonymous methods were in Java first.

Java introduced anonymous classes in Java 1.1 in 1997.

Delphi introduced anonymous methods in 2009.

Java introduced anonymous lambdas/closures in Java 8 in 2014 (but they were first proposed in 2006).

Edited by Remy Lebeau

Share this post


Link to post
On 7/27/2024 at 6:30 PM, Remy Lebeau said:

Delphi introduced anonymous methods in 2009

What specific feature are you talking about here? 

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×