Skip to content

Comments

Node bridge memory cleanup#208

Merged
aramikm merged 1 commit intomainfrom
finalization_node_registry
Aug 12, 2024
Merged

Node bridge memory cleanup#208
aramikm merged 1 commit intomainfrom
finalization_node_registry

Conversation

@aramikm
Copy link
Collaborator

@aramikm aramikm commented Aug 8, 2024

Goal

The goal of this PR is to implement an automated memory cleanup mechanism that might help free the memory in cases that the uses forget to manually free it.

Closes #128

Copy link
Collaborator

@JoeCap08055 JoeCap08055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good--have you done any testing to see if the finalizer actually does anything?

@aramikm
Copy link
Collaborator Author

aramikm commented Aug 9, 2024

I've done testing but was not able to trigger that. It seems like it's not easy triggering it in Node. If you have any ideas how we might be able to force it to trigger I'm open to ideas.

@saraswatpuneet
Copy link
Contributor

I was not able to reproduce it either , it has weird behavior and unpredictable it will be worth running a load test and let it wait to see if it ever gets triggered

@aramikm
Copy link
Collaborator Author

aramikm commented Aug 9, 2024

@JoeCap08055 @saraswatpuneet I was able to verify it by directly calling global.gc inside an app which triggers cleanup.
Running following code with following command

node --expose-gc dist/index.js
function interactWithGraph() {
    const environment: EnvironmentInterface =  {environmentType: EnvironmentType.Mainnet};
    for(let i=0;i<10; i++) {
        const graph= new Graph(environment);

        let public_follow_graph_schema_id = graph.getSchemaIdFromConfig(environment, ConnectionType.Follow, PrivacyType.Public);

        let connect_action: ConnectAction = {
            type: "Connect",
            ownerDsnpUserId: "1",
            connection: {
                dsnpUserId: "2",
                schemaId: public_follow_graph_schema_id,
            } as Connection,
            dsnpKeys: {
                dsnpUserId: "2",
                keysHash: 100,
                keys: [],
            } as DsnpKeys,
        } as ConnectAction;

        let actions = [] as Action[];
        actions.push(connect_action);

        let applied = graph.applyActions(actions);
        console.log(applied);

        let connections_including_pending = graph.getConnectionsForUserGraph("1", public_follow_graph_schema_id, true);
        console.log(connections_including_pending);

        let exported = graph.exportUpdates();
        console.log(exported);
    }
    if (global.gc) {
        global.gc();
    } else {
        console.log('Garbage collection unavailable.  Pass --expose-gc '
            + 'when launching node to enable forced garbage collection.');
    }
}

which printed

freeing 8
freeing 7
freeing 6
freeing 5
freeing 4
freeing 3
freeing 2
freeing 1
freeing 0

@saraswatpuneet
Copy link
Contributor

Thats a cool thing to know 🥇

Copy link
Contributor

@saraswatpuneet saraswatpuneet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽

@aramikm aramikm merged commit 451043b into main Aug 12, 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 this pull request may close these issues.

[graph-sdk] Use JS finalizer registration to handle freeing graph state

3 participants