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

Add an example sending chunked responses #82

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mfelsche
Copy link
Contributor

FYI @redvers

Let me know if this example addresses your problem.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 29, 2025
@redvers
Copy link

redvers commented Jan 30, 2025

The issue as I understand it is that finished() is called by actor _ServerConnection here:

  be _receive_finished(request_id: RequestID) =>
    """
    Indicates that the last *inbound* body chunk has been sent to
    `_chunk`. This is passed on to the back end.
    """
    _backend.finished(request_id)

In your example, (Line 211-212):

    while true do
      let file_chunk = file.read(chunk_size)

All the Array[U8 val] iso's that you create will be new allocations and will never have any opportunity to be available for Garbage Collection until:

a. The loop has completed (In other words, the entire size of the file has to have been fully allocated to memory).
b. The _receive_finished() behaviour has terminated.

I don't think it solves the problem, as it still results in at least 4.7G of memory allocation for a 4.7G .iso file.

@redvers
Copy link

redvers commented Jan 30, 2025

After receiving the completed request, we need to send the chunks across multiple behaviours to allow for GC to happen.

in order to not load the whole file into memory
@mfelsche
Copy link
Contributor Author

mfelsche commented Feb 3, 2025

I solved that issue by delegating the actual file reading and sending to another actor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss during sync Should be discussed during an upcoming sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants