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

PackageNotFoundErrors use significant amount of memory #43

Open
pcdavid opened this issue Jun 14, 2024 · 0 comments · May be fixed by #45
Open

PackageNotFoundErrors use significant amount of memory #43

pcdavid opened this issue Jun 14, 2024 · 0 comments · May be fixed by #45
Assignees

Comments

@pcdavid
Copy link
Member

pcdavid commented Jun 14, 2024

This might be partially related to the Sirius Web context... but it's the main use case in practice.

#40 changed PackageNotFoundException into a plain class to avoid the runtime cost of creating many exceptions. The result is much better in terms of speed, but we still create a lot of instances of this error, most (all?) of them with the exact same data.

After loading the "Sirius Web" Papaya example, the total retained size of all JsonResourceImpl is about 21MB, almost 1/4th of it in 48795 instances of PackageNotFoundError:

image

An analysis of the duplicate strings shows that whenever an inter-resource reference is parsed, because we try to interpret the target resource URI as an EPackage nsURI first, and fail in 99.9% of the cases, we create a new String with the resource URI (all/most of the UUID-like strings in the screenshot below) and wrap it in a new PackageNotFoundError in each case:

image

Possible solutions:

  1. Stop creating these PackageNotFoundError, or at least make this optional.
  2. Clear them after load (on the Sirius Web side), if we do not actually use them for anything.
  3. Deduplicate them.
  4. Stop trying to resolve external resource URIs as EPackage (a very rare case in practice) as the default. From my understanding, for non-null input, GsonEObjectDeserializer.getPackageForURI(String) will always end up calling EMF's URI.createURI(uriString) which itself will end up in org.eclipse.emf.common.util.URI.URIPool.StringAccessUnit.parseIntoURI(String). I think a valid URI will always contain a : scheme separator, but that is never the case for the names/URIs of Sirius Web documents (which use UUIDs), so we could probably bypass the whole getPackageForURI method (and the corresponding errors) when the uriString does not contain : (which should be fast to test).
pcdavid added a commit that referenced this issue Jun 24, 2024
This avoids creating thousands of PackageNotFoundError instances when
resolving "normal" inter-document references which do not refer to
EPackages via nsURIs.

PackageNotFoundError are much cheaper to create than the previously
used PackageNotFoundException, but that's still a lot of garbage
instances to create, add into the resource.getErrors() list and never
use (or even clear).

In addition, in the context of Sirius Web, every instance will have
its own duplicate string representation of the failed "package" URI
because we get it from EObjectIDManager.getOrCreateId(EObject) which
invokes UUID.toString() and allocates a new (identical) String each
time.

Bug: #43
Signed-off-by: Pierre-Charles David <[email protected]>
@pcdavid pcdavid linked a pull request Jun 24, 2024 that will close this issue
@pcdavid pcdavid linked a pull request Jun 24, 2024 that will close this issue
@pcdavid pcdavid self-assigned this Jun 24, 2024
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.

1 participant