Skip to content

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

@jemc
Copy link
Member

jemc commented May 13, 2025

We discussed this on the sync call. The basic outlook here is that both Sean and Red would like to see a refactored API for this library in the future, but Red says that for now this PR is good documentation of the API that currently exists, and it makes sense to merge.

@jemc jemc merged commit cfb7e75 into main May 13, 2025
8 checks passed
@jemc jemc deleted the chunked-response-example branch May 13, 2025 18:45
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label May 13, 2025
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.

5 participants