1
FPC development / Improvement of TFPTimerThread.Execute
« Last post by lagprogramming on Today at 12:53:29 pm »packages/fcl-base/src/fptimer.pp has
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.
- {$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.