Skip to content

Avoid unnecessary array copy before calling .Remap(); new() -> ValueTuple; #191

Merged
alex-kulakov merged 2 commits intoDataObjects-NET:masterfrom
servicetitan:upstream/Optimize_ValueTuple
Nov 24, 2021
Merged

Avoid unnecessary array copy before calling .Remap(); new() -> ValueTuple; #191
alex-kulakov merged 2 commits intoDataObjects-NET:masterfrom
servicetitan:upstream/Optimize_ValueTuple

Conversation

@SergeiPavlov
Copy link
Copy Markdown
Contributor

@SergeiPavlov SergeiPavlov commented Nov 12, 2021

  • Refactor .Equals() methods
  • Change Select(..., params int[]) arguments type -> Select(..., IReadOnlyList<int>) to avoid conversions

…uple; Pair<> (#48)

* Avoid unnecessary array copy before calling .Remap(); new() -> ValueTuple; Pair<>

* rename x -> other in .Equals() method

* rename to 'other'
@alex-kulakov
Copy link
Copy Markdown
Contributor

@SergeiPavlov, I see that ColumnGatherer.GetColumns() changed return type to IEnumerable which forced a lot of places to make .ToList() or ToArray() but in other places. Did it worth it? What scenario overweight these enumerations + possible number of Array.Copy? Please, point me.

@SergeiPavlov
Copy link
Copy Markdown
Contributor Author

SergeiPavlov commented Nov 19, 2021

@SergeiPavlov, I see that ColumnGatherer.GetColumns() changed return type to IEnumerable which forced a lot of places to make .ToList() or ToArray() but in other places. Did it worth it? What scenario overweight these enumerations + possible number of Array.Copy? Please, point me.

There are a few places whe we need not to call neither .ToList() nor .ToArray(), but just to enumerate:

          var columns = orderByProjector
            .GetColumns(ColumnExtractionModes.TreatEntityAsKey | ColumnExtractionModes.Distinct);
          foreach (var c in columns) {
            if (!sortColumns.ContainsKey(c)) {
              sortColumns.Add(c, direction);
            }
          }
      var keySegment = visitedSource.ItemProjector.GetColumns(ColumnExtractionModes.TreatEntityAsKey);
      var keyPairs = keySegment
        .Select((leftIndex, rightIndex) => new Pair<int>(leftIndex, rightIndex))
        .ToArray();

I removed .ToList() from .GetColumns() implementation. So total number of list allocations has been decreased

@alex-kulakov
Copy link
Copy Markdown
Contributor

@SergeiPavlov If we had both results would it be better?

@alex-kulakov alex-kulakov merged commit 34b30ed into DataObjects-NET:master Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants