search

temporalio / sdk-core

Core Temporal SDK that can be used as a base for language specific Temporal SDKs
MIT License
266 stars 71 forks source link

[Feature Request] Drop all payloads from state machines as soon as they are not required #363

Open bergundy opened 2 years ago

bergundy commented 2 years ago

This is done in Java SDK, could lead to significant memory savings in some cases.

Sushisource commented 1 year ago

This is already true in pretty much any case where it matters, and we'd want some meaningful perf data before spending time here. Just gonna close this and if it ever is a measurable problem later we can deal with it then.

bergundy commented 1 year ago

Reopening this with some more context so we keep this tracked.

Take for example the activity state machine. In Core we keep the input around for the lifetime of the state machine (which could be the duration of the workflow) compared to Java where we drop it once it's no longer needed (in some cases): https://github.com/temporalio/sdk-java/blob/7521f859398623e453506b365b8542f3743ec74a/temporal-sdk/src/main/java/io/temporal/internal/statemachines/ActivityStateMachine.java#L413

See also the Java child workflow state machine. https://github.com/temporalio/sdk-java/blob/7521f859398623e453506b365b8542f3743ec74a/temporal-sdk/src/main/java/io/temporal/internal/statemachines/ChildWorkflowStateMachine.java#L185

I 100% agree with @Sushisource that we should measure memory consumption and get better at finding the right optimizations. For now this has not come up as a high priority but it is also a quick win since we know that there's potentially large payloads in these inputs and we don't have a reason for keeping them around.

bergundy commented 1 year ago

We discovered that we don't drop the state machines at all today, we should consider fixing that too, not just dropping the payloads.