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

Rework GitStatusProvider to use cached status storage #10690

Merged
merged 5 commits into from Aug 29, 2018
Merged

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Aug 7, 2018

What does this PR do?

JGit status command on big projects (Eclipse Che) uses a lot of RAM.
screenshot from 2018-08-22 09-54-27_

When deep expand to some folder is performed, Git status is called on each folder expand:
screenshot-172 17 0 1-8080-2018 08 22-10-24-12

This causes a huge leap of RAM and finally OOM appears:
screenshot from 2018-08-22 09-58-43

In this PR GitStatusProvider is reworked to call Git status only if it is really needed e.g. file was modified. The provider is based on a cached Git Status Map that is updated from Git Events.

What issues does this PR fix or reference?

#10696

Release Notes

Fix java.lang.OutOfMemoryError on large projects.

Docs PR

N/A

@vinokurig vinokurig added kind/bug Outline of a bug - must adhere to the bug report template. status/in-progress This issue has been taken by an engineer and is under active development. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Aug 8, 2018
@vinokurig vinokurig changed the title CHE-10543: Switch git status to cached Status in GitStatusProvider [WIP] CHE-10543: Switch git status to cached Status in GitStatusProvider Aug 8, 2018
@vinokurig vinokurig changed the title [WIP] CHE-10543: Switch git status to cached Status in GitStatusProvider Rework GitStatusProvider to use cached status storage Aug 22, 2018
@vinokurig vinokurig added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Aug 22, 2018
@@ -1,119 +0,0 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't remove this class for now, just mark as Deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

collectProjectFiles(rootDirPathProvider.get());
}

private void collectProjectFiles(String root) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I propose add @PostConstruct and remove call this method from constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@vinokurig
Copy link
Contributor Author

ci-test

@riuvshin
Copy link
Contributor

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:10690
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@vinokurig vinokurig merged commit 3f93f58 into master Aug 29, 2018
@vinokurig vinokurig deleted the CHE-10543 branch August 29, 2018 05:59
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Aug 29, 2018
@benoitf benoitf added this to the 6.11.0 milestone Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants