FixInsight and the inline directive
The Delphi compiler allows functions and procedures to be tagged with the inline directive to improve performance. If the function or procedure meets certain criteria, the compiler will insert code directly, rather than generating a call. Embarcadero docwiki gives a list of conditions under which inlining does or does not occur.
One of basic conditions says: within a unit, the body for an inline function should be defined before calls to the function are made.
FixInsight 2017.04 introduces rule O805 “Inline marked routine comes after its call in the same unit”. Let’s check FMX and VCL code to see if Embarcadero follows their own rules. Short answer: it doesn’t.
I will give a couple of examples from Delphi 10.1 Berlin (version 24.0.22858.6822).
procedure TWinControl.ReadState(Reader: TReader); begin DisableAlign; try inherited ReadState(Reader); finally EnableAlign; end; FixupTabList; if FParent <> nil then Perform(CM_PARENTCTL3DCHANGED, 0, 0); UpdateControlState; end;
procedure TWinControl.AlignControl(AControl: TControl); var Rect: TRect; begin if not HandleAllocated or (csDestroying in ComponentState) then Exit; if FAlignLevel <> 0 then Include(FControlState, csAlignmentNeeded) else begin DisableAlign; try Rect := GetClientRect; AlignControls(AControl, Rect); finally Exclude(FControlState, csAlignmentNeeded); EnableAlign; end; end; end;
procedure TWinControl.DisableAlign; begin Inc(FAlignLevel); end;
TWinControl.DisableAlign is an inline marked procedure. It is called at line 8531 and line 9019, but its body is defined after the calls – at line 9030. Obviously, this function will not be inlined.
One more example from another unit:
procedure TListViewBase.UpdateDeleteButtonLayout; var RelRect: TRectF; begin if (Adapter.Count < 1) or (FDeleteLayout = nil) or ((FDeleteButtonIndex = -1) and (FPrevDeleteButtonIndex = -1)) then Exit; if (FListingService <> nil) and (TListingTransitionFeature.DeleteButtonSlide in FListingService.GetTransitionFeatures) then begin FDeleteLayout.Width := DefaultDeleteButtonWidth * FDeleteModeTransitionAlpha; FDeleteButton.Opacity := 1; end else begin if FDeleteModeTransitionAlpha > 0 then FDeleteLayout.Width := DefaultDeleteButtonWidth else FDeleteLayout.Width := 0; FDeleteButton.Opacity := 0.5 + (FDeleteModeTransitionAlpha / 2); end; FDeleteLayout.Height := GetItemHeight(FDeleteButtonIndex); FDeleteLayout.Position.X := Width - FDeleteLayout.Width; if FDeleteButtonIndex = -1 then RelRect := GetItemRelRect(FPrevDeleteButtonIndex, LocalRect) else RelRect := GetItemRelRect(FDeleteButtonIndex, LocalRect); FDeleteLayout.Position.Y := (RelRect.Top + RelRect.Bottom - FDeleteLayout.Height) / 2; end;
The method above contains two GetItemRelRect calls (lines 2457 and 2459), but both are before the actual GetItemRelRect body position in that unit (line 2868):
function TListViewBase.GetItemRelRect(const Index: Integer; const LocRect: TRectF; const SideSpace: Integer = 0): TRectF; begin Result := RectF(LocRect.Left + FSideSpace + SideSpace, LocRect.Top + FSideSpace + FHeightSums[Index] - FScrollViewPos, LocRect.Width - ((SideSpace + FSideSpace) * 2), GetItemHeight(Index)); if (FScrollBar <> nil) and (not HasTouchTracking) and FScrollBar.Visible then Result.Right := Result.Right - FScrollBar.Width; end;
Despite being declared as inline, this method will not be inlined.
It is not a critical issue, but this makes inline directive useless. There are more occurrences of this issue in Vcl.ExtActns.pas, FMX.ASE.Lexer.pas, FMX.Graphics.pas, FMX.Types.pas, FMX.Utils.pas and FMX.ZOrder.Win.pas.
This means that inlining conditions are not easy to follow, even though at first glance inline directive seems to be an easy way to slightly optimize your code. FixInsight may help to make inlining more useful by avoiding such mistakes.
You can download FixInsight trial and check your own code.
P.S. Thanks to Stefan Glienke for suggesting this new FixInsight rule.
Here I have some suggestion. You should add checking all objects used in Destructors are checked for nil as many people are not aware that if exception is thrown in constructor then destroy is called immediately and many fields/objects might be not initialized. The only exception is Free method that can be called on NIL object. BTW please fix some language mistakes like “Suspect FREE call” message should be “Suspicious FREE call”. I found also some number of false positive warnings. Anyway, great product as it is, definitely going to buy it.