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

fix: oom on long messages #449

Merged
merged 8 commits into from
Sep 6, 2024

Conversation

njhale
Copy link
Member

@njhale njhale commented Sep 6, 2024

  • fix: stop OOM on long messages
  • fix: use workqueue to offload progress events

Addresses: #403

As currently implemented, this still has the socket disconnect problem.

Signed-off-by: Donnie Adams <donnie@acorn.io>
@njhale njhale force-pushed the fix-oom-on-long-messages-w-queue branch from 16b1087 to facefe0 Compare September 6, 2024 02:44
Offload the processing of progress events to a workequeue to free up
resources for socket.io's heartbeat (i.e. ping/pong) to work. This
prevents the page from refreshing on long running responses.

Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
@njhale njhale force-pushed the fix-oom-on-long-messages-w-queue branch from facefe0 to f6bafb1 Compare September 6, 2024 16:20
@njhale njhale marked this pull request as ready for review September 6, 2024 16:21
…g interval

Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
thedadams
thedadams previously approved these changes Sep 6, 2024
components/chat/useChatSocket.tsx Outdated Show resolved Hide resolved
components/chat.tsx Show resolved Hide resolved
components/chat/messages.tsx Show resolved Hide resolved
components/chat/messages/confirmForm.tsx Show resolved Hide resolved
components/chat/useChatSocket.tsx Show resolved Hide resolved
Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
@njhale njhale requested a review from thedadams September 6, 2024 19:07
Signed-off-by: Donnie Adams <donnie@acorn.io>
thedadams
thedadams previously approved these changes Sep 6, 2024
Copy link
Contributor

@tylerslaton tylerslaton left a comment

Choose a reason for hiding this comment

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

I didn't mean to approve, and I have another issue I'm seeing. I submitted the review too early, but comments coming in a moment.

@tylerslaton
Copy link
Contributor

I'm seeing this behavior with Confirm calls, Prompt calls and general tool calls.

Screen.Recording.2024-09-06.at.4.01.58.PM.mov

Signed-off-by: Donnie Adams <donnie@acorn.io>
Copy link
Contributor

@tylerslaton tylerslaton left a comment

Choose a reason for hiding this comment

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

All of my comments and issues are addressed, looking forward to getting this through.

@njhale njhale merged commit 0feebf9 into gptscript-ai:main Sep 6, 2024
1 check passed
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.

4 participants