Skip to content

Fix performance regression caused by #5465: daemon part #5556

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

Merged
merged 2 commits into from
Sep 3, 2018

Conversation

ilevkivskyi
Copy link
Member

This is part of the original PR #5544 that takes care of the daemon performance regression caused by #5465. (I separated it from original PR as requested by @JukkaL.)

I will add tests I promised in #5544 (daemon variants of the tests added in #5465 and #5544).

This change should not affect semantics, only performance by avoiding cloning options for lots of modules in situation with --follow-imports=skip.

@ilevkivskyi
Copy link
Member Author

@JukkaL I added the tests that you asked (just fine grained versions of previously added normal incremental tests). It looks like there is only one problematic test but it is not affected by any of the recent changes.

I will make performance measurements later on Monday.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for fixing all the tests! I suspect that this is easy to break accidentally without good test coverage.

Can you add an issue about the remaining test failure? It seems like a pretty high-priority thing to fix.

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