Visitar URL original
Optimize performance of IncludeMemberByPathSelectionRule by jnyrup · Pull Request #969 · fluentassertions/fluentassertions · GitHub
Skip to content

Optimize performance of IncludeMemberByPathSelectionRule - #969

Merged
jnyrup merged 1 commit into
fluentassertions:masterfrom
jnyrup:memberPath
Nov 17, 2018
Merged

Optimize performance of IncludeMemberByPathSelectionRule #969
jnyrup merged 1 commit into
fluentassertions:masterfrom
jnyrup:memberPath

Conversation

@jnyrup

@jnyrup jnyrup commented Nov 16, 2018

Copy link
Copy Markdown
Member

This PR has three parts:

  • Lazy-loading Segments instead of eagerly in constructor
  • Caching segments for candidates, by wrapping them as a MemberPath
  • Use type check as fail-fast in IsSameAs

@dennisdoomen

dennisdoomen commented Nov 16, 2018

Copy link
Copy Markdown
Member

What are you trying to accomplish with this PR?

@jnyrup

jnyrup commented Nov 16, 2018

Copy link
Copy Markdown
Member Author

It triggered me that:

  • the candidate memberpath was segmentized 1-3 times in IncludeMemberByPathSelectionRule, while we can do with atmost 1
  • segments were eagerly computed, where I prefer to minimize the work in the constructor
  • the segments were (to me) unnecessarily copied into a List.
  • the candidate memberpath was segmentized before the declaringTypes are compared.

I think IncludeMemberByPathSelectionRule becomes prettier

@dennisdoomen dennisdoomen changed the title lazy and caching GetSegments Optimize performance of IncludeMemberByPathSelectionRule Nov 17, 2018
@jnyrup
jnyrup merged commit fe56239 into fluentassertions:master Nov 17, 2018
@jnyrup
jnyrup deleted the memberPath branch November 17, 2018 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dennisdoomen dennisdoomen dennisdoomen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jnyrup @dennisdoomen