maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Improved ATTR_ROW::ReplaceAttrs performance (#6573)

Browse source 
## Summary of the Pull Request

Improve `ATTR_ROW::ReplaceAttrs` performance by only reserving the necessary capacity instead of resizing the new run.
That way `TextAttributeRun`s are only instantiated once instead of twice.

## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

Performance could be further improved by directly moving `TextAttributeRun`s into the new vector, but I considered this out of scope for this PR.

## Validation Steps Performed

CPU usage when running `cacafire` is slightly reduced.
This commit is contained in:
Leonard Hecker 2020-06-19 17:55:17 +02:00 committed by GitHub
parent acd3ba7bd9
commit 4f55568a17
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 22 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
src/buffer/out/AttrRow.cpp
View file

@ -264,15 +264,16 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
{
// First we try to find the run where the insertion happens, using lowerBound and upperBound to track
// where we are currently at.
const auto begin = _list.begin();
size_t lowerBound = 0;
size_t upperBound = 0;
for (size_t i = 0; i < _list.size(); i++)
{
upperBound += _list.at(i).GetLength();
const auto curr = begin + i;
upperBound += curr->GetLength();
if (iStart >= lowerBound && iStart < upperBound)
{
const auto curr = std::next(_list.begin(), i);
// The run that we try to insert into has the same color as the new one.
// e.g.
// AAAAABBBBBBBCCC
@ -385,7 +386,7 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// fact that an existing piece of the run was split in half (to hold the latter half).
const size_t cNewRun = _list.size() + newAttrs.size() + 1;
std::vector<TextAttributeRun> newRun;
newRun.resize(cNewRun);
newRun.reserve(cNewRun);
// We will start analyzing from the beginning of our existing run.
// Use some pointers to keep track of where we are in walking through our runs.
@ -393,10 +394,9 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// Get the existing run that we'll be updating/manipulating.
const auto existingRun = _list.begin();
auto pExistingRunPos = existingRun;
const auto pExistingRunEnd = existingRun + _list.size();
const auto pExistingRunEnd = _list.end();
auto pInsertRunPos = newAttrs.begin();
size_t cInsertRunRemaining = newAttrs.size();
auto pNewRunPos = newRun.begin();
size_t iExistingRunCoverage = 0;
// Copy the existing run into the new buffer up to the "start index" where the new run will be injected.
@ -410,7 +410,7 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
iExistingRunCoverage += pExistingRunPos->GetLength();
// Copy it to the new run buffer and advance both pointers.
*pNewRunPos++ = *pExistingRunPos++;
newRun.push_back(*pExistingRunPos++);
}
// When we get to this point, we've copied full segments from the original existing run
@ -429,12 +429,8 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// We need to fix this up below so it says G2 instead to leave room for the Y3 to fit in
// the new/final run.
// Copying above advanced the pointer to an empty cell beyond what we copied.
// Back up one cell so we can manipulate the final item we copied from the existing run to the new run.
pNewRunPos--;
// Fetch out the length so we can fix it up based on the below conditions.
size_t length = pNewRunPos->GetLength();
size_t length = newRun.back().GetLength();
// If we've covered more cells already than the start of the attributes to be inserted...
if (iExistingRunCoverage > iStart)
@ -449,7 +445,7 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// Now we're still on that "last cell copied" into the new run.
// If the color of that existing copied cell matches the color of the first segment
// of the run we're about to insert, we can just increment the length to extend the coverage.
if (pNewRunPos->GetAttributes() == pInsertRunPos->GetAttributes())
if (newRun.back().GetAttributes() == pInsertRunPos->GetAttributes())
{
length += pInsertRunPos->GetLength();
@ -460,18 +456,11 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
}
// We're done manipulating the length. Store it back.
pNewRunPos->SetLength(length);
// Now that we're done adjusting the last copied item, advance the pointer into a fresh/blank
// part of the new run array.
pNewRunPos++;
newRun.back().SetLength(length);
}
// Bulk copy the majority (or all, depending on circumstance) of the insert run into the final run buffer.
std::copy_n(pInsertRunPos, cInsertRunRemaining, pNewRunPos);
// Advance the new run pointer into the position just after everything we copied.
pNewRunPos += cInsertRunRemaining;
std::copy_n(pInsertRunPos, cInsertRunRemaining, std::back_inserter(newRun));
// We're technically done with the insert run now and have 0 remaining, but won't bother updating its pointers
// and counts any further because we won't use them.
@ -488,9 +477,6 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// If we still have original existing run cells remaining, copy them into the final new run.
if (pExistingRunPos != pExistingRunEnd || iExistingRunCoverage != (iEnd + 1))
{
// Back up one cell so we can inspect the most recent item copied into the new run for optimizations.
pNewRunPos--;
// We advanced the existing run pointer and its count to on or past the end of what the insertion run filled in.
// If this ended up being past the end of what the insertion run covers, we have to account for the cells after
// the insertion run but before the next piece of the original existing run.
@ -514,11 +500,11 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// This case is slightly off from the example above. This case is for if the B2 above was actually Y2.
// That Y2 from the existing run is the same color as the Y2 we just filled a few columns left in the final run
// so we can just adjust the final run's column count instead of adding another segment here.
if (pNewRunPos->GetAttributes() == pExistingRunPos->GetAttributes())
if (newRun.back().GetAttributes() == pExistingRunPos->GetAttributes())
{
size_t length = pNewRunPos->GetLength();
size_t length = newRun.back().GetLength();
length += (iExistingRunCoverage - (iEnd + 1));
pNewRunPos->SetLength(length);
newRun.back().SetLength(length);
}
else
{
@ -526,14 +512,14 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// its length for the discrepancy in columns not yet covered by the final/new run.
// Move forward to a blank spot in the new run
pNewRunPos++;
newRun.emplace_back();
// Copy the existing run's color information to the new run
pNewRunPos->SetAttributes(pExistingRunPos->GetAttributes());
newRun.back().SetAttributes(pExistingRunPos->GetAttributes());
// Adjust the length of that copied color to cover only the reduced number of columns needed
// now that some have been replaced by the insert run.
pNewRunPos->SetLength(iExistingRunCoverage - (iEnd + 1));
newRun.back().SetLength(iExistingRunCoverage - (iEnd + 1));
}
// Now that we're done recovering a piece of the existing run we skipped, move the pointer forward again.
@ -551,34 +537,25 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// New Run desired when done = R3 -> B7
// Existing run pointer is on B2.
// We want to merge the 2 from the B2 into the B5 so we get B7.
else if (pNewRunPos->GetAttributes() == pExistingRunPos->GetAttributes())
else if (newRun.back().GetAttributes() == pExistingRunPos->GetAttributes())
{
// Add the value from the existing run into the current new run position.
size_t length = pNewRunPos->GetLength();
size_t length = newRun.back().GetLength();
length += pExistingRunPos->GetLength();
pNewRunPos->SetLength(length);
newRun.back().SetLength(length);
// Advance the existing run position since we consumed its value and merged it in.
pExistingRunPos++;
}
// OK. We're done inspecting the most recently copied cell for optimizations.
pNewRunPos++;
// Now bulk copy any segments left in the original existing run
if (pExistingRunPos < pExistingRunEnd)
{
std::copy_n(pExistingRunPos, (pExistingRunEnd - pExistingRunPos), pNewRunPos);
// Fix up the end pointer so we know where we are for counting how much of the new run's memory space we used.
pNewRunPos += (pExistingRunEnd - pExistingRunPos);
std::copy_n(pExistingRunPos, (pExistingRunEnd - pExistingRunPos), std::back_inserter(newRun));
}
}
// OK, phew. We're done. Now we just need to free the existing run, store the new run in its place,
// and update the count for the correct length of the new run now that we've filled it up.
newRun.erase(pNewRunPos, newRun.end());
// OK, phew. We're done. Now we just need to free the existing run and store the new run in its place.
_list.swap(newRun);
return S_OK;