View Issue Details

IDProjectCategoryView StatusLast Update
0034320LazarusLazUtilspublic2019-02-08 22:59
ReporterSerge AnvarovAssigned ToMattias Gaertner 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformWindowsOSOS Version
Product VersionProduct Build 
Target VersionFixed in Version 
Summary0034320: Implementation of LazStringUtils.ConvertLineEndings can be removed (optimization)
DescriptionConvertLineEndings does the same thing as LineBreaksToSystemLineBreaks, so implementation can removed
Patch added
Additional InformationThis test shows that the new implementation works even faster
[code]
{$APPTYPE CONSOLE}
{$MODE OBJFPC}{$H+}

uses SysUtils, LazStringUtils;

const
  CRepeatCount = 500000;
type
  TMeasureFunction = function(const S: string): string;

procedure Measure(const Desc, TestLine: string; Func: TMeasureFunction);
var
  S: string;
  i: Integer;
  Start, Elapsed: QWord;
begin
  Start := GetTickCount64;
  for i := 1 to CRepeatCount do
    S := Func(TestLine);
  Elapsed := GetTickCount64 - Start;
  Writeln(Desc, ' elapsed - ', Elapsed);
end;

function ConvertLineEndingsNew(const S: string): string;
begin
  Result := LineBreaksToSystemLineBreaks(S);
end;

const
  CTestLine1 = 'Line'0000013'Line'0000013;
  CTestLine2 = 'Line' + LineEnding + 'Line' + LineEnding;
begin
  Measure('Old CTestLine1', CTestLine1, @ConvertLineEndings);
  Measure('New CTestLine1', CTestLine1, @ConvertLineEndingsNew);
  Measure('New CTestLine2', CTestLine2, @ConvertLineEndingsNew);
  Measure('Old CTestLine2', CTestLine2, @ConvertLineEndings);
  Readln;
end.
[/code]
On my computer (Win x64):
[Result]
Old CTestLine1 elapsed - 266
New CTestLine1 elapsed - 46
New CTestLine2 elapsed - 32
Old CTestLine2 elapsed - 109
[/Result]
TagsNo tags attached.
Fixed in Revision59142 59144
LazTarget-
Widgetset
Attached Files
  • SimpleConvertLineEndings.diff (1,700 bytes)
    Index: components/lazutils/lazstringutils.pas
    ===================================================================
    --- components/lazutils/lazstringutils.pas	(revision 59116)
    +++ components/lazutils/lazstringutils.pas	(working copy)
    @@ -11,7 +11,7 @@
     }
     unit LazStringUtils;
     
    -{$mode objfpc}{$H+}
    +{$mode objfpc}{$H+} {$D+}
     
     interface
     
    @@ -42,7 +42,7 @@
     function ChangeLineEndings(const s, NewLineEnding: string): string;
     function LineBreaksToSystemLineBreaks(const s: string): string;
     function LineBreaksToDelimiter(const s: string; Delimiter: char): string;
    -function ConvertLineEndings(const s: string): string;
    +function ConvertLineEndings(const s: string): string; inline;
     
     // Conversions
     function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;
    @@ -188,28 +188,8 @@
     end;
     
     function ConvertLineEndings(const s: string): string;
    -var
    -  i: Integer;
    -  EndingStart: LongInt;
     begin
    -  Result:=s;
    -  i:=1;
    -  while (i<=length(Result)) do begin
    -    if Result[i] in [#10,#13] then begin
    -      EndingStart:=i;
    -      inc(i);
    -      if (i<=length(Result)) and (Result[i] in [#10,#13])
    -      and (Result[i]<>Result[i-1]) then
    -        inc(i);
    -      if (length(LineEnding)<>i-EndingStart)
    -      or (LineEnding<>copy(Result,EndingStart,length(LineEnding))) then begin
    -        // line end differs => replace with current LineEnding
    -        Result:=copy(Result,1,EndingStart-1)+LineEnding+copy(Result,i,length(Result));
    -        i:=EndingStart+length(LineEnding);
    -      end;
    -    end else
    -      inc(i);
    -  end;
    +  Result := LineBreaksToSystemLineBreaks(s);
     end;
     
     function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;
    
  • SimpleConvertLineEndings2.diff (1,591 bytes)
    Index: components/lazutils/lazstringutils.pas
    ===================================================================
    --- components/lazutils/lazstringutils.pas	(revision 59116)
    +++ components/lazutils/lazstringutils.pas	(working copy)
    @@ -42,7 +42,7 @@
     function ChangeLineEndings(const s, NewLineEnding: string): string;
     function LineBreaksToSystemLineBreaks(const s: string): string;
     function LineBreaksToDelimiter(const s: string; Delimiter: char): string;
    -function ConvertLineEndings(const s: string): string;
    +function ConvertLineEndings(const s: string): string; inline;
     
     // Conversions
     function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;
    @@ -188,28 +188,8 @@
     end;
     
     function ConvertLineEndings(const s: string): string;
    -var
    -  i: Integer;
    -  EndingStart: LongInt;
     begin
    -  Result:=s;
    -  i:=1;
    -  while (i<=length(Result)) do begin
    -    if Result[i] in [#10,#13] then begin
    -      EndingStart:=i;
    -      inc(i);
    -      if (i<=length(Result)) and (Result[i] in [#10,#13])
    -      and (Result[i]<>Result[i-1]) then
    -        inc(i);
    -      if (length(LineEnding)<>i-EndingStart)
    -      or (LineEnding<>copy(Result,EndingStart,length(LineEnding))) then begin
    -        // line end differs => replace with current LineEnding
    -        Result:=copy(Result,1,EndingStart-1)+LineEnding+copy(Result,i,length(Result));
    -        i:=EndingStart+length(LineEnding);
    -      end;
    -    end else
    -      inc(i);
    -  end;
    +  Result := LineBreaksToSystemLineBreaks(s);
     end;
     
     function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;
    

Activities

Serge Anvarov

2018-09-21 21:48

reporter  

SimpleConvertLineEndings.diff (1,700 bytes)
Index: components/lazutils/lazstringutils.pas
===================================================================
--- components/lazutils/lazstringutils.pas	(revision 59116)
+++ components/lazutils/lazstringutils.pas	(working copy)
@@ -11,7 +11,7 @@
 }
 unit LazStringUtils;
 
-{$mode objfpc}{$H+}
+{$mode objfpc}{$H+} {$D+}
 
 interface
 
@@ -42,7 +42,7 @@
 function ChangeLineEndings(const s, NewLineEnding: string): string;
 function LineBreaksToSystemLineBreaks(const s: string): string;
 function LineBreaksToDelimiter(const s: string; Delimiter: char): string;
-function ConvertLineEndings(const s: string): string;
+function ConvertLineEndings(const s: string): string; inline;
 
 // Conversions
 function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;
@@ -188,28 +188,8 @@
 end;
 
 function ConvertLineEndings(const s: string): string;
-var
-  i: Integer;
-  EndingStart: LongInt;
 begin
-  Result:=s;
-  i:=1;
-  while (i<=length(Result)) do begin
-    if Result[i] in [#10,#13] then begin
-      EndingStart:=i;
-      inc(i);
-      if (i<=length(Result)) and (Result[i] in [#10,#13])
-      and (Result[i]<>Result[i-1]) then
-        inc(i);
-      if (length(LineEnding)<>i-EndingStart)
-      or (LineEnding<>copy(Result,EndingStart,length(LineEnding))) then begin
-        // line end differs => replace with current LineEnding
-        Result:=copy(Result,1,EndingStart-1)+LineEnding+copy(Result,i,length(Result));
-        i:=EndingStart+length(LineEnding);
-      end;
-    end else
-      inc(i);
-  end;
+  Result := LineBreaksToSystemLineBreaks(s);
 end;
 
 function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;

Serge Anvarov

2018-09-22 08:36

reporter  

SimpleConvertLineEndings2.diff (1,591 bytes)
Index: components/lazutils/lazstringutils.pas
===================================================================
--- components/lazutils/lazstringutils.pas	(revision 59116)
+++ components/lazutils/lazstringutils.pas	(working copy)
@@ -42,7 +42,7 @@
 function ChangeLineEndings(const s, NewLineEnding: string): string;
 function LineBreaksToSystemLineBreaks(const s: string): string;
 function LineBreaksToDelimiter(const s: string; Delimiter: char): string;
-function ConvertLineEndings(const s: string): string;
+function ConvertLineEndings(const s: string): string; inline;
 
 // Conversions
 function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;
@@ -188,28 +188,8 @@
 end;
 
 function ConvertLineEndings(const s: string): string;
-var
-  i: Integer;
-  EndingStart: LongInt;
 begin
-  Result:=s;
-  i:=1;
-  while (i<=length(Result)) do begin
-    if Result[i] in [#10,#13] then begin
-      EndingStart:=i;
-      inc(i);
-      if (i<=length(Result)) and (Result[i] in [#10,#13])
-      and (Result[i]<>Result[i-1]) then
-        inc(i);
-      if (length(LineEnding)<>i-EndingStart)
-      or (LineEnding<>copy(Result,EndingStart,length(LineEnding))) then begin
-        // line end differs => replace with current LineEnding
-        Result:=copy(Result,1,EndingStart-1)+LineEnding+copy(Result,i,length(Result));
-        i:=EndingStart+length(LineEnding);
-      end;
-    end else
-      inc(i);
-  end;
+  Result := LineBreaksToSystemLineBreaks(s);
 end;
 
 function TabsToSpaces(const s: string; TabWidth: integer; UseUTF8: boolean): string;

Serge Anvarov

2018-09-22 08:37

reporter   ~0110948

Sorry, I forgot to remove the debug directive. New patch

Bart Broersma

2018-09-22 11:47

developer   ~0110949

Why not
  Result := ChangeLineEndings(s,LineEnding);
1 function call less.

Serge Anvarov

2018-09-23 11:00

reporter   ~0110970

1. Function LineBreaksToSystemLineBreaks alread exists and called.
2. The call point for ChangeLineEndings (inlining) will generate a larger code - a function call with two parameters.
3. Within the meaning of the function ChangeLineEndings is more appropriate (and more commonly used) when NewLineEnding <> LineEnding. As for the standard case uses LineBreaksToSystemLineBreaks.

Bart Broersma

2018-09-23 12:08

developer   ~0110972

Why not deprecate ConvertLineEndings, so it can be removed later?

Mattias Gaertner

2018-09-23 12:34

manager   ~0110974

I deprecated it.

Serge Anvarov

2019-02-08 22:59

reporter   ~0113960

In Release

Issue History

Date Modified Username Field Change
2018-09-21 21:48 Serge Anvarov New Issue
2018-09-21 21:48 Serge Anvarov File Added: SimpleConvertLineEndings.diff
2018-09-22 08:36 Serge Anvarov File Added: SimpleConvertLineEndings2.diff
2018-09-22 08:37 Serge Anvarov Note Added: 0110948
2018-09-22 11:47 Bart Broersma Note Added: 0110949
2018-09-23 08:42 Mattias Gaertner Fixed in Revision => 59142
2018-09-23 08:42 Mattias Gaertner LazTarget => -
2018-09-23 08:42 Mattias Gaertner Status new => resolved
2018-09-23 08:42 Mattias Gaertner Resolution open => fixed
2018-09-23 08:42 Mattias Gaertner Assigned To => Mattias Gaertner
2018-09-23 11:00 Serge Anvarov Note Added: 0110970
2018-09-23 12:08 Bart Broersma Note Added: 0110972
2018-09-23 12:08 Bart Broersma Status resolved => assigned
2018-09-23 12:08 Bart Broersma Resolution fixed => reopened
2018-09-23 12:34 Mattias Gaertner Fixed in Revision 59142 => 59142 59144
2018-09-23 12:34 Mattias Gaertner Note Added: 0110974
2018-09-23 12:34 Mattias Gaertner Status assigned => resolved
2018-09-23 12:34 Mattias Gaertner Resolution reopened => fixed
2019-02-08 22:59 Serge Anvarov Note Added: 0113960
2019-02-08 22:59 Serge Anvarov Status resolved => closed