Remove massive string allocations #5940
Conversation
|
@forki Done thanks |
|
@forki thanks you right now the LineEndPositions is according to the source and not the normalized source I think now it's ok |
| [| | ||
| let mutable pos = 0 | ||
| yield pos | ||
| for i in 0..source.Length-1 do |
There was a problem hiding this comment.
i and pos are always in sync right? So you can get rid of that mutable variable
|
ok so now I need to fix the tests |
|
It's green |
|
Thank you |
| let c = source.[i] | ||
| if c = '\r' && i + 1 < source.Length && source.[i+1] = '\n' then () | ||
| elif c = '\r' then yield i + 1 | ||
| if c = '\n' then yield i + 1 |
There was a problem hiding this comment.
Should not this be an elif? (It does not change the semantics but improves clarity.) @AviAvni
There was a problem hiding this comment.
I think it's fine. alter native would be to pattern match on c
| yield 0 | ||
| for i in 0..source.Length-1 do | ||
| let c = source.[i] | ||
| if c = '\r' && i + 1 < source.Length && source.[i+1] = '\n' then () |
There was a problem hiding this comment.
style/readbility comment: looping through 1..source.Length instead of 0..source.Length-1 would make more sense here, since we already processed the 0th byte with the yield 0 above the loop. This would also avoid the i+1 in four places:
[|
yield 0
for i in 1..source.Length do
let c = source.[i-1]
if c = '\r' && i < source.Length && source.[i] = '\n' then ()
elif c = '\r' then yield i
elif c = '\n' then yield i
yield source.Length
|]There was a problem hiding this comment.
yeah I think we should avoid duplicates as well.
| { NormalizedSource = source | ||
| LineEndPositions = positions }) | ||
| [| | ||
| yield 0 |
| |> Seq.toArray | ||
| { NormalizedSource = source | ||
| LineEndPositions = positions }) | ||
| [| |
There was a problem hiding this comment.
Using Seq.pairwise might be more elegant here, rahter than indexing in arrays. (Though I'm not sure if perf would be better or worse taking into account possible compiler optimization and CPU caching).
| @@ -433,8 +433,7 @@ let _ = printf " %*a" 3 (fun _ _ -> ()) 2 | |||
| typeCheckResults.GetFormatSpecifierLocationsAndArity() | |||
| |> Array.map (fun (range,numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs) | |||
| |> shouldEqual | |||
| [|(2, 45, 2, 47, 1); (3, 23, 3, 25, 1); (4, 38, 4, 40, 1); (5, 27, 5, 29 | |||
| , 1); | |||
| [|(2, 45, 2, 47, 1); (3, 23, 3, 25, 1); (4, 38, 4, 40, 1); (5, 27, 5, 29, 1); | |||
There was a problem hiding this comment.
can you explain this change?
There was a problem hiding this comment.
Just remove line break
|
this is amazeballs
|
See #5922