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

Investigate possible memory leaks #34

Open
37 of 82 tasks
LSViana opened this issue Sep 16, 2023 · 6 comments · Fixed by #54
Open
37 of 82 tasks

Investigate possible memory leaks #34

LSViana opened this issue Sep 16, 2023 · 6 comments · Fixed by #54
Assignees

Comments

@LSViana
Copy link
Collaborator

LSViana commented Sep 16, 2023

In #18, there has been a mention of possible memory leaks in the operations executed through YDotNet.

This issue intends to:

  1. Investigate the possible issues
  2. Document the discoveries
  3. Create a procedure and, possibly, an automated way to detect such issues in the future

Investigated scenarios (this list is not final):

  • Doc
    • Create instances without options
    • Create instances with options
    • Read Id
    • Read Guid
    • Read CollectionId
    • Read ShouldLoad
    • Read AutoLoad
    • Invoke Clone() and Dispose()
    • Observe clear (leaking)
    • Observe updates V1 (leaking)
    • Observe updates V2 (leaking)
    • Observe after transaction (leaking)
    • Observe subdocs (leaking)
  • Text
    • Create instances
    • Insert text
    • Insert embed
    • Remove text
    • Format text
    • Read the chunks
    • Read the string representation
    • Read the length
    • Observe changes (leaking)
  • Array
    • Create instances
    • Insert range
    • Remove range
    • Get by index
    • Move by index
    • Iterate (leaking)
    • Read the length
    • Observe changes (leaking)
  • Map
    • Create instances
    • Insert
    • Read by key
    • Remove by key
    • Remove all
    • Iterate
    • Read length
    • Observe changes (leaking)
  • XmlText
    • Create instances
    • Insert text by index
    • Remove text by index
    • Format text by index
    • Insert embed by index
    • Insert attribute
    • Read attribute
    • Remove attribute
    • Iterate
    • Read as string
    • Read the previous sibling
    • Read the next sibling
    • Read the length
    • Observe changes (leaking)
  • XmlElement
    • Create instances
    • Read tag
    • Read as string
    • Insert attribute
    • Read attribute
    • Remove attribute
    • Iterate
    • Read child length
    • Insert text by index
    • Insert element by index
    • Remove items by index
    • Read item by index
    • Read the previous sibling
    • Read the next sibling
    • Read parent element
    • Read first child
    • Walk the tree
    • Observe changes (leaking)
  • Branch
    • Read a sticky index
    • Observe deep (leaking)
    • Create read transaction
    • Create write transaction

Other fixes:

  • Change subscriptions to be stored in their parent collections to prevent Delegate disposal
@LSViana LSViana converted this from a draft issue Sep 16, 2023
@LSViana LSViana self-assigned this Sep 17, 2023
@LSViana
Copy link
Collaborator Author

LSViana commented Sep 17, 2023

First round of investigations:

  1. Creating documents without explicitly disposing them leaves the memory disposal for the GC, which can cause a very high memory usage until the operating system memory is close to exhaustion (this behavior was seen in both Windows and macOS) but it's still freed as expected
  2. Creating documents and explicitly disposing them has constant memory usage

For instance, this screenshot demonstrates the scenario (2):
image

Further investigations will be conducted for other operations.

@LSViana
Copy link
Collaborator Author

LSViana commented Sep 19, 2023

Second round of investigations:

  1. It was discovered that creating Doc instances with DocOptions that included string properties would cause an increasing usage of memory until the Dispose() method of DocOptionsNative was called
  2. This was improved to call that Dispose() method manually and now the memory usage to create Doc instances with DocOptions is constant too

@LSViana
Copy link
Collaborator Author

LSViana commented Sep 20, 2023

Third round of investigations:

  1. To prevent relying on clients of the library to invoke Dispose() on resources, I'm implementing destructors on public classes exposed by the library.
  2. The private or internal classes or structs shouldn't implement Dispose() since it's expected that the library, internally, calls Dispose() correctly wherever it's needed.

@SebastianStehle
Copy link
Collaborator

I think input will also leak memory. e.g. when creating a new collection input a new block of memory is allocated and just the pointer is given to y-crdt. But the pointer is never released. I think a finalizer is needed here.

@SebastianStehle
Copy link
Collaborator

@LSViana Can we close this?

@LSViana
Copy link
Collaborator Author

LSViana commented Jul 8, 2024

@SebastianStehle No, the initial work I did here was useful for detecting and fixing memory leaks.

Given the previous discoveries, I believe it's important to check for possible leaks in the pending items too. It's not a complex task, it's more laborious and repetitive, I'll try to do a little bit every day.

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 a pull request may close this issue.

2 participants