Reduce TextDiffer allocations, including LOH#11983
Conversation
TextDiffer.ComputeDiff shows as 1.3% of allocations in the razor speedometer scrolling and typing test. There are two issues: 1) The IntArray class was using too large a page size when used in conjunction with System.Buffers.ArrayPool. It was indicating it wanted an 80KB buffer, and ArrayPool allocates it's pool entries on powers of 2, so it was giving back an 128KB array, which goes to the LOH. The fix is to request a 64 KB buffer. 2) The IntArrays were being constructed over unnecessarilly large ranges. If we instead prime the pump by detemining an initial changed extent, we can request significantly less allocated space during the typing scenario (reduces the requested array size from the buffer size to just the changed range, tyipcally only a couple characters) Reducing the size of the vf/vr arrays did require a redirection to the SourceEqual method in the derived classes, as they don't know of that concept.
| using var vr = new IntArray((2 * max) + 1); | ||
|
|
||
| ComputeDiffRecursive(builder, 0, OldSourceLength, 0, NewSourceLength, vf, vr); | ||
| ComputeDiffRecursive(builder, 0, oldSourceLength, 0, newSourceLength, vf, vr); |
There was a problem hiding this comment.
I suspect the answer is buried in the complicated algorithm I'm refusing to look at, but why pass 0 here only to then apply an offset later, instead of just computing the diff starting from the offset?
There was a problem hiding this comment.
I tried using an offset to index into the vf/vr vectors in, but that got logically quite hairy, and I still had bugs after trying that for a couple hours. It seemed easier to just say:
algorithm, do your thing over this smaller region. But, when you need to call SourceEqual, let me intersept that call and do the translation. And when you are done, I'll translate your answer to use the appropriate coordinate space.
There was a problem hiding this comment.
I would have done the same. I hate these highly mathematical algorithms with tiny variable names. I didn't get into this business to do math.
| { | ||
| var edit = edits[i]; | ||
|
|
||
| edits[i] = new DiffEdit(edit.Kind, _oldSourceOffset + edit.Position, _newSourceOffset + edit.NewTextPosition, edit.Length); |
There was a problem hiding this comment.
Since there was effort to make the DiffEdit constructor private and provide factory methods, I'd prefer an Offset method that takes adjustments to DiffEdit.Position and DiffEdit.NewTextPosition rather than exposing the constructor and passing half of the same data back in.
Specifically, provide a factory method for offseting a DiffEdit instead of exposing the DiffEdit ctor
4 participants
TextDiffer.ComputeDiff shows as 1.3% of allocations in the razor speedometer scrolling and typing test. There are two issues:
The IntArray class was using too large a page size when used in conjunction with System.Buffers.ArrayPool. It was indicating it wanted an 80KB buffer, and ArrayPool allocates it's pool entries on powers of 2, so it was giving back an 128KB array, which goes to the LOH. The fix is to request a 64 KB buffer.
The IntArrays were being constructed over unnecessarilly large ranges. If we instead prime the pump by detemining an initial changed extent, we can request significantly less allocated space during the typing scenario (reduces the requested array size from the buffer size to just the changed range, tyipcally only a couple characters)
Reducing the size of the vf/vr arrays did require a redirection to the SourceEqual method in the derived classes, as they don't know of that concept.