Improvement of TFPTimerThread.Execute

from 'lagprogramming'.

packages/fcl-base/src/fptimer.pp has

    {$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.

patch.diff