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

IPROTO-155 Reduce FileDescriptor memory usage #226

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tristantarrant
Copy link
Member

https://issues.redhat.com/browse/IPROTO-155

Parse annotations eagerly to avoid keeping them around.

* Use ArrayList instead of LinkedList
* Resolve annotations eagerly
@tristantarrant tristantarrant added this to the 5.0.0.Dev02 milestone Feb 7, 2024

private void calculateSize(Map<String, FileDescriptor> descriptors) {
for (Map.Entry<String, FileDescriptor> descriptor : descriptors.entrySet()) {
System.out.println("Total Size = " + GraphLayout.parseInstance(descriptor.getValue()).totalSize());
Copy link
Member

Choose a reason for hiding this comment

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

always log this as debug?

Copy link
Contributor

@fax4ever fax4ever left a comment

Choose a reason for hiding this comment

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

Just a suggestion here.
I'm also testing the changes on current Infinispan main.

* @return the map of annotations
* @throws AnnotationParserException if parsing of annotations fails
*/
Map<String, AnnotationElement.Annotation> getAnnotations() throws AnnotationParserException;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can keep the method implemented as as default simpy delegating to getProcessedAnnotation("")

@fax4ever
Copy link
Contributor

fax4ever commented Feb 9, 2024

Sorry @tristantarrant I only noticed running this patch on current Infinispan main, that there are some cases where the annotations field is filled and processedAnnotations not. This probably could be an issue for the remote query code. Full story here => https://infinispan.zulipchat.com/#narrow/stream/118645-infinispan/topic/pr.20queue/near/420641654.

An idea to keep the optimization, is to fill the annotations list only if:

annotations ⊈ processedAnnotations

at the end of processAnnotations methods. WTDY?

fax4ever added a commit to fax4ever/infinispan that referenced this pull request Feb 22, 2024
@tristantarrant tristantarrant modified the milestones: 5.0.0.CR1, 5.0.8.Final Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants