Skip to content

Replace Work.sourceValues List<ValueEntity> with lightweight ID references to reduce memory retention #178

Description

@stalep

Problem

Work.sourceValues is List<ValueEntity>, meaning every queued Work item carries full ValueEntity objects — including their JqValue data field (the parsed JSON tree). During bulk imports, cascade Work items are created for each dependent node and queued with the same sourceValues list. With rhivos runs at 3.2 MB per upload and 134 nodes per run, dozens of queued Work items each hold a reference to the same heavy data.

The WorkService.execute() method already reloads source values from the database at the start of every execution (lines 246-257):

List<ValueEntity> sourceValues = new ArrayList<>();
for (ValueEntity sv : w.sourceValues) {
    if (Hibernate.isPropertyInitialized(sv, "data")) {
        sourceValues.add(sv);
    } else {
        ValueEntity managed = em.find(ValueEntity.class, sv.id);
        Hibernate.initialize(managed.data);
        sourceValues.add(managed);
    }
}

This means the full ValueEntity objects in Work.sourceValues are only needed for their .id during reload — the actual data is always re-fetched from the DB (or 2LC cache). Carrying full entities through the queue wastes memory.

Heap dump evidence

Heap dump analysis during a 20-test import showed:

  • 79,074 Object[] references to ProxyJqObject instances held via Work objects in the queue
  • 34,590 Object[] references to ValueEntity instances retained by Hibernate event listeners processing Work-held entities

Current Work.sourceValues usage analysis

Location Properties Accessed Works with IDs?
Work constructor List copy only YES
Work.dependsOn() line 73 .node (NodeEntity ref) NO — needs node ID
Work.dependsOn() line 75 .dependsOn(ValueEntity) — walks value DAG NO — needs entity graph
Work.dependsOn() line 84 .isEmpty() YES
Work.dependsOn() line 88 .contains() via equals().id YES
Work.equals() .equals().id YES
Work.hashCode() .hashCode()id, node, folder YES (simpler)
Work.toString() .getId() YES
WorkService.findTrackers() .id YES
WorkService.execute() reload .id for em.find() YES — simpler
WorkService.execute() cascade Passes list to new Work() YES — extract IDs
FolderService.upload() Passes entity to new Work() YES — use .getId()
ProcessingService Passes entity to new Work() YES — use .id

The blocker: Work.dependsOn() lines 71-79

for (int i = 0, size = sourceValues.size(); i < size; i++) {
    ValueEntity sourceValue = sourceValues.get(i);
    // Needs sourceValue.node to check node-level dependency
    if (anyNodeDependsOn(work.activeNodes, sourceValue.node)) {
        // Walks the ValueEntity.sources DAG (value_edge table)
        for (int j = 0, wSize = work.sourceValues.size(); j < wSize; j++) {
            if (sourceValue.dependsOn(work.sourceValues.get(j))) {
                return true;
            }
        }
    }
}

This block accesses two things that require full entities:

  1. sourceValue.node — the node that produced this value (to check if any activeNode in the other Work depends on it)
  2. sourceValue.dependsOn(otherValue) — BFS walk of the value_edge source graph

The original developer comment on line 65 says: "needs reviewing for the value dependency", suggesting uncertainty about whether this check is needed.

Proposed solutions

Option A: record SourceRef(long valueId, long nodeId)

Replace List<ValueEntity> with List<SourceRef> where SourceRef carries just the value ID and its owning node ID.

  • Satisfies the .node check on line 73 — NodeEntity.hasAncestor(sourceRef.nodeId) is O(1) via the ancestor cache
  • The ValueEntity.dependsOn() call (line 75) would need to be either:
    • Replaced with a recursive SQL query on value_edge
    • Removed if deemed unnecessary (see Option B)

Pros: Minimal semantic change, keeps the value-level dependency check, memory-lightweight
Cons: Still needs to resolve the ValueEntity.dependsOn() DAG walk

Option B: Remove the value-level dependency check (lines 71-79)

The value-level dependency check may be redundant given that:

  • Lines 81-92 already check node-level dependency (anyNodeDependsOnAny(this.activeNodes, work.activeNodes))
  • Lines 87-89 check value equality (work.sourceValues.contains(sourceValues.get(i)))
  • Work ordering is determined by the node DAG, not the value DAG
  • If node A depends on node B, then work for A is already ordered after work for B regardless of which specific values they process
  • The value-DAG walk only adds information when two Work items process different uploads but share values through the value_edge graph — which is the common case for change detection nodes that aggregate historical data

With this block removed, sourceValues could become List<Long> directly.

Pros: Simplest solution, eliminates the entire blocker, dependsOn() becomes purely node-based
Cons: May change ordering semantics — needs careful review of whether any scenario relies on value-level dependency ordering

Option C: Replace ValueEntity.dependsOn() with a recursive SQL query

Keep List<Long> for sourceValues, but when dependsOn() needs to check value ancestry, execute a recursive CTE:

WITH RECURSIVE ancestors AS (
    SELECT parent_id FROM value_edge WHERE child_id = ?
    UNION ALL
    SELECT ve.parent_id FROM value_edge ve JOIN ancestors a ON ve.child_id = a.parent_id
)
SELECT 1 FROM ancestors WHERE parent_id = ?

Pros: Correct semantics, no entities needed
Cons: Adds DB round-trips inside dependsOn(), which is called O(n²) times during sort — could make performance worse. Would need caching.

Recommendation

Option A with a fallback to Option B is the safest path:

  1. Start with record SourceRef(long valueId, long nodeId) to eliminate the heavy ValueEntity data retention
  2. For the ValueEntity.dependsOn() call, assess whether it can be removed (Option B) or needs SQL replacement (Option C)
  3. The developer comment "needs reviewing" suggests this is a good opportunity to evaluate whether the value-level check adds correctness value beyond the node-level check

Impact

This change would eliminate the primary remaining source of memory retention during bulk imports — queued Work items carrying multi-megabyte parsed JSON trees through the work queue. Combined with the existing Work.run() nulling (issue #175), this would ensure Work objects are memory-lightweight throughout their lifecycle, not just after execution.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions