Skip to content

[Perf] Every container object retains the whole raw JSON tree alongside the DTOs built from it #46

Description

@egg82

What problem are you trying to solve?

AbstractFetchableAPIObject keeps the parsed tree for the object's entire lifetime, in addition to the resource list its constructor already built from it:

Lib/src/main/java/me/egg82/arr/common/AbstractFetchableAPIObject.java:12

protected final JsonNode node;

So a Movie holding 31,053 MovieResource objects also holds the JSON those were parsed out of, for as long as it's reachable.

Measured on a 65 MB Whisparr cache file, retained heap after a full GC: 259-268 MB as shipped, 97-104 MB with the tree dropped - so the raw tree is 61-62% of the live set. Two independent harnesses agreed on the ratio.

The leaf DTOs already made the opposite call - AbstractAPIObject.java:15 gates the equivalent field behind PROVIDE_RAW_API_OBJ and defaults it to null. Confirmed by reflection on a live object: MovieResource.obj is null while Movie.node wraps a populated tree.

The tree is discarded once the DTOs are built, or retained only when something asks for it.

The trap - do not just null the field

node() is not only a plugin accessor. writeCache calls it two lines after construction:

AbstractArrAPI.java:112   writeCache(composite, params);
AbstractArrAPI.java:276     cacheFile.write(composite.node());

(and the same pair at :167 / :314 for the by-id variant)

A bare null-out fails loudly - node() is new JsonNode(node.toString()), so it NPEs. The dangerous version is the one that looks most natural: copying the null-safe pattern from next door, AbstractAPIObject.obj()'s new JSONObject(obj != null ? obj.toMap() : Map.of()). Do that and node() returns an empty tree, cacheFile.write() writes an empty cache file, every subsequent read is a miss, and the cache is silently corrupted. That is worse than the memory it saves.

equals, hashCode and toString at lines 43-59 also reference node directly, so they need rework either way.

What would you like Fetcharr to do?

The tree is needed exactly once, immediately, for the cache write. writeCache only uses composite for getBasePath(composite.getClass()), composite.node() and composite.lastFetched() - so pass those in directly rather than pulling the tree back off the object, and the field never needs to exist.

That removes the retention and the node() deep copy on the write path in one change, since you'd be handing it the tree you already have.

Note node() is on the public FetchableAPIObject interface, so whatever replaces it is a contract change for plugin authors.

Optional additional context or use case that could be helpful

The same profiling run recommended dropping the container memory limit to 1Gi on the strength of a probe that measured "flat, no restarts" there. That probe ran with dry-run enabled and an empty cache, so it never built this DTO graph - it measured the resting set, not the working set. Tried against the real workload, a 1Gi limit gives a 768MB heap and the Whisparr cycle exits with OutOfMemoryError: Java heap space.

The retained-heap figures above are from a different harness and are not affected. But the limit only becomes reducible after this and the projection work land - not before.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions