Lots of allocations and time spent in LexFilter

[TIHan]

The results are from when I did a trace with this PR: #6001

Now that source files are not showing up in the traces anymore, we can start to see what's coming up next in our list.

These traces were done when making a single modification in the middle of TypeChecker.fs, around line 6699 then waiting for semantic classification to kick in as well as errors.

GC Heap Allocs

Cpu Stacks

--

The next traces below are the same thing, except I removed calls to Tokenizer.getClassifiedSpans in some of our analyzers.

GC Heap Allocs

Cpu Stacks

It seemed to make a slight bit of a difference, but LexFilter and interpret still show up at top in Cpu Stacks and TokenTup (it's inside LexFilter) in GC Heap Allocs.

My fear was that calls to Tokenizer.getClassifiedSpans were slowing things down due to brace completions and syntactic classification.

[gerardtoconnor]

Hi @TIHan , just to log our slack discussion, my proposed changes in lexfilter.fs to structures of TokenTup, LexbufState & PositionTuple yielded about 33% allocation improvement time, about 3% to overall time. It's not a final solution but does trim a third off the execution times of TokenTup. I changed LexbufState & PositionTuple to Class objects, and the TokenTup to a Struct as it was being splintered all over the place having to do heavy struct copies of the LexbufState & PositionTuple ( 20 ints and a bool copy in LexbufState & 10 int copies for PositionTuple.

Can I do a PR for this or do we need further investigation?

//[<Struct>]  << no longer Struct
type LexbufState(startPos: Position, 
                 endPos  : Position, 
                 pastEOF : bool) = 
    member x.StartPos = startPos
    member x.EndPos = endPos
    member x.PastEOF = pastEOF

//[<Struct>] << no longer Struct
type PositionTuple =
    val X: Position
    val Y: Position
    new (x: Position, y: Position) = { X = x; Y = y }

/// Used to save the state related to a token
//[<Class>] << no longer class
[<Struct>]
type TokenTup = 
    val Token : token
    val LexbufState : LexbufState
    val LastTokenPos: PositionTuple
    new (token,state,lastTokenPos) = { Token=token; LexbufState=state;LastTokenPos=lastTokenPos }

[forki]

In any case a PR is a good thing - it would run full test suite and show if it's valid optimization

[gerardtoconnor]

👍 Ok, will do, I have few other ideas how to do small fringe improvements, I can include also if yielding further benefit in my crude benchmark. I'm not home till late but will try do at some stage today.

[cartermp]

@gerardtoconnor Yes, please do submit a PR. We can tweak things from there but incremental improvements like this end up moving the needle meaningfully over time.

[TIHan]

A PR will be fine. We should evaluate what you changed and get evidence of better performance. Judging by what you recorded, it looks like it was an improvement.

[gerardtoconnor]

PR #6010 made to apply these changes … I ended up restructuring Position also so that the overhead is now only 2% vs our original 9% … but for some reason, other calls have come down too and reduced the base so technically improvement is better (or test case usage is now smaller somehow!!?). If @TIHan can validate the improvement would be great.

[chinwobble]

@TIHan Is it possible you could point to some documentation on how to generate and analyse these traces?

[cartermp]

@chinwobble You can take a look here: https://blogs.msdn.microsoft.com/dotnet/2012/10/09/improving-your-apps-performance-with-perfview/

That will get you going. Once you get this sort of info, interpreting it does require some understanding of the codebase and what should show up as producing a lot of allocations/lots of CPU time or not.

[TIHan]

Closing as we solved the big issue with allocations of TokenTup a while back by implementing a pool.