Improvement of TFPTimerThread.Execute
lazarus / 64-FPC development

----------------------------------------------------------------------
lagprogramming · 2024-04-18 10:53

packages/fcl-base/src/fptimer.pp has
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i {$ifdef
Has_EventWait}procedure TFPTimerThread.Execute;var  WakeTime,
StartTime: TDateTime;  WakeInterval: Integer;  Counter: int64; { use
Int64 to avoid overflow with Counter*fInterval (~49 days)}  AInterval:
int64;  Diff: Extended;  Const  wrSignaled = 0;  wrTimeout  = 1; 
wrAbandoned= 2;  wrError    = 3;  begin  WakeInterval := MaxInt; 
Counter := 1;  AInterval := fInterval;  FStartTime := Now;  while not
Terminated do    begin    if
GetWakeTime(AInterval,Counter,WakeInterval,WakeTime) then      
Continue;    if not Terminated then      case
BasicEventWaitFor(WakeInterval,fWaitEvent) of      wrTimeout:       
begin        if Terminated then          Break        else         
begin          try            if not Timer.UseTimerThread then       
     // If terminate is called while here, then the Synchronize will
be              // queued while the stoptimer is being processed.    
        // StopTimer cannot wait until thread completion as this would
deadlock              Synchronize(@Timer.Timer)  // Call user event  
        else              Timer.Timer;          except            //
Trap errors to prevent this thread from terminating          end;    
    Inc(Counter);                // Next interval          end;      
end;      wrSignaled:        begin        if Terminated then         
Break        else           begin                      // Interval has
changed          Counter := 1;              // Restart timer without
creating new thread          AInterval := fInterval;         
FStartTime := Now;           end;        end;      else        Break; 
    end    end;  BasicEventDestroy(fWaitEvent);end; {$ELSE
Has_EventWait} procedure TFPTimerThread.Execute; var  WakeTime,
StartTime: TDateTime;  WakeInterval: Integer;  Counter: int64; { use
Int64 to avoid overflow with Counter*fInterval (~49 days)}  AInterval:
int64;  Diff: Extended;  S,Last: Cardinal;  RecheckTimeCounter:
integer;  const  cSleepTime = 500;           // 0.5 second, better
than every 5 milliseconds  cRecheckTimeCount = 120;    // Recheck
clock every minute, as the sleep loop can loose time  begin 
WakeInterval := MaxInt;  Counter := 1;  AInterval := fInterval; 
FStartTime := Now;  while not Terminated do    begin    if
GetWakeTime(AInterval,Counter,WakeInterval,WakeTime) then     
Continue;    if not Terminated then      begin      RecheckTimeCounter
:= cRecheckTimeCount;      s := cSleepTime;      repeat        if s >
WakeInterval then          s := WakeInterval;        sleep(s);       
if fSignaled then            // Terminated or interval has changed   
     begin          if not Terminated then            begin          
fSignaled := False;            Counter := 1;            // Restart
timer            AInterval := fInterval;            StartTime := Now; 
          end;          break;                     // Need to break
out of sleep loop          end;         dec(WakeInterval,s);        
// Update total wait time        dec(RecheckTimeCounter);     // Do we
need to recheck current time        if (RecheckTimeCounter < 0) and
(WakeInterval > 0) then          begin          Diff := (WakeTime -
Now);          WakeInterval := Trunc(Diff * cMilliSecs);         
RecheckTimeCounter := cRecheckTimeCount;          s := cSleepTime;   
     end;      until (WakeInterval<=0) or Terminated;      if
WakeInterval <= 0 then        try          inc(Counter);          if
not Timer.UseTimerThread then            // If terminate is called
while here, then the Synchronize will be            // queued while
the stoptimer is being processed.            // StopTimer cannot wait
until thread completion as this would deadlock           
Synchronize(@Timer.Timer)  // Call user event          else          
Timer.Timer;        except          // Trap errors to prevent this
thread from terminating        end;      end    end;end;{$ENDIF
Has_EventWait}




Variable "Last" is declared but never used, which means it can be
removed.
Looking at variable StartTime I've noticed that when Has_EventWait is
defined, the variable can be removed because it's not used at all. But
then I've looked at the content of the procedure when Has_EventWait is
not defined. It has just a simple assignment: StartTime := Now;. When
Has_EventWait is defined the equivalent line is FStartTime := Now;. It
doesn't look right. Looks like a bug, a typo. Most likely should have
been "FStartTime := Now;" instead of "StartTime := Now;".
Here is a patch.

----------------------------------------------------------------------
AlexTP · 2024-04-18 15:56

Posted to https://gitlab.com/freepascal.org/fpc/source/-/issues/40744