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

[Improvement]: Improve EnvironmentPackageCache to Cache Only Required Data #40398

Closed
hasithaa opened this issue May 11, 2023 · 4 comments · Fixed by #40559
Closed

[Improvement]: Improve EnvironmentPackageCache to Cache Only Required Data #40398

hasithaa opened this issue May 11, 2023 · 4 comments · Fixed by #40559
Assignees
Milestone

Comments

@hasithaa
Copy link
Contributor

Description

Currently, all the data, including syntax trees and documents, is cached in the project API for later use. However, when dealing with a large dependency tree, this can result in out-of-memory errors (OOM). Initially, these data are cached to be used by LS and other tools. However, it has been observed that LS does not utilize this cache and instead relies on the Compiler API to derive the required information. Even the resolution of 'Go to Definition' is handled differently. After discussions with @azinneera and @IMS94, we concluded that it would be worth exploring the possibility of cleaning up unnecessary data once the BIR is generated.

Here is a heap dump generated for the following Ballerina program.

import ballerina/io;
import wso2healthcare/healthcare.fhir.r4;
import wso2healthcare/healthcare.hl7v23;
import wso2healthcare/healthcare.hl7v231;
import wso2healthcare/healthcare.hl7v24;
import wso2healthcare/healthcare.hl7v25;
import wso2healthcare/healthcare.hl7v251;
import wso2healthcare/healthcare.hl7v26;
Screenshot 2023-05-10 at 16 34 48

Describe your problem(s)

No response

Describe your solution(s)

No response

Related area

-> Compilation

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

@sameerajayasoma
Copy link
Contributor

@hasithaa, I had a quick look at this. The compiler API heavily uses the cached SyntaxTree object. We can't simply remove this. It will slow down the compilation and LS operations. So your original claim is wrong :)

@hasithaa
Copy link
Contributor Author

@sameerajayasoma this is only happening at when we compile from Bala. In other cases, we use bir and we don't have syntax trees to cache . My original argument for those depending packages' cached syntax trees. Those are not used by after generating the bir. If it's we can't really compile from the BIRs.

@nirmal070125
Copy link
Contributor

nirmal070125 commented May 30, 2023

One workaround to this problem is increasing the max memory of the JVM.
Steps:

  1. Locate {bal_home} -
    In OS X;
% where bal                                                            
/Library/Ballerina/bin/bal

Hence,
{bal_home}=/Library/Ballerina/

Note:
In Linux;

% realpath bal                                                            
/usr/lib/ballerina/bin/bal
  1. Find active Ballerina version -
% bal --version
Ballerina 2201.4.2 (Swan Lake Update 4)
Language specification 2022R4
Update Tool 1.3.14

Hence,
{version}=2201.4.2

  1. Open the bal file to edit -

{bal_home}/distributions/ballerina-{version}/bin/bal

  1. Locate Xmx value and increase it to 3GB.
-Xmx3g

@Thevakumar-Luheerathan
Copy link
Member

As per the discussion, we have decided following things,

  1. Syntax Tree caching for the dependency modules is disabled for a package as those syntax trees are not much usable.
  2. Introduce a hidden command line flag (something like --disable-syntax-tree-caching) for CLI commands to suppress the syntax tree generation for the sources(these sources itself are enough to cause the OOM) as these syntax tree is not much usable during CLI command execution.(whenever we need syntax tree of a source, It will be generated with this flag)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants