Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Inefficient use of entity framework in GetAllWithUsersAndCollaboratorsAsync() (in the ProjectRepository) #376

Closed
TimSnoek123 opened this issue Feb 23, 2021 · 7 comments · Fixed by #380
Assignees
Labels
bug Something isn't working Refactor Friday This label is used to mark an issue for a planned refactor

Comments

@TimSnoek123
Copy link
Contributor

Describe the bug
The function GetAllWithUsersAndCollaboratorsAsync() in the ProjectRepository implements a really inefficient way of getting data. This should be improved by using .include(). With 5000 projects using .include() takes around 300 ms and the current way of getting the data takes around 1900 ms. (and even with less data it is around 70% faster)

Screenshots
image
As you can see above for every project 2 additional queries are called in order to get the collaborators and the likes. Using include only executes one query which is way faster especially when getting lots of data.

@TimSnoek123 TimSnoek123 added the bug Something isn't working label Feb 23, 2021
@TimSnoek123 TimSnoek123 self-assigned this Feb 23, 2021
@TimSnoek123 TimSnoek123 changed the title Inefficient use of entity framework in GetAllWithUsersAndCollaboratorsAsync() (in the ProjectRepository) Bug: Inefficient use of entity framework in GetAllWithUsersAndCollaboratorsAsync() (in the ProjectRepository) Feb 23, 2021
@RubenFricke RubenFricke added the Refactor Friday This label is used to mark an issue for a planned refactor label Feb 23, 2021
@Brend-Smits
Copy link
Member

This is very strange. The results I got when testing this were the other way around. Are you sure the data set that you tested with includes likes and collaborators?
See: #335

@TimSnoek123
Copy link
Contributor Author

TimSnoek123 commented Feb 23, 2021

The data did have collaborators but not likes, I will try testing it again tomorrow I guess with more relational data.

@Brend-Smits
Copy link
Member

The data did have collaborators but not likes, I will try testing it again tomorrow I guess with more relational data.

Still, the results should be the other way around... unless something has changed in terms of packages that got updated to fix this issue.
How many collaborators were included per project? (Average)

@TimSnoek123
Copy link
Contributor Author

I just created 5000 projects each with: 7 collaborators and one like.

The include took around: 7 seconds.
The old version took around: 23 seconds.

@Brend-Smits
Copy link
Member

Well, that sounds good then! I'm glad to hear the issue is resolved. Nice😀

@TimSnoek123
Copy link
Contributor Author

Yes! Maybe Microsoft updated EF or something ¯_(ツ)_/¯

@niraymak niraymak added this to To do in Sprint 9 - Backend via automation Feb 24, 2021
@RamonPeek RamonPeek self-assigned this Feb 25, 2021
@TimSnoek123 TimSnoek123 moved this from To do to In progress in Sprint 9 - Backend Feb 25, 2021
@RamonPeek
Copy link
Contributor

Just validated the behaviour on my machine. Old code takes about 70ms to execute for 30 projects (with collaborators), and the improved code takes about 15 to 20 ms. I also included the likes and Redacted user, so only one SQL query has to be executed each time the projects are retrieved. This second include takes another 3 to 5ms from the response time.

I'll push the changes and open a PR for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Refactor Friday This label is used to mark an issue for a planned refactor
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants