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

WIP: experimentally use dupIO (take 2) #321

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

adamgundry
Copy link
Member

@adamgundry adamgundry commented May 24, 2023

This is an alternative to #319 aimed at fixing #318.

It calls dupIO on the token stream each time the buffer is full, meaning that any thunks in the old generation will be copied into the new generation relatively quickly, but we do not incur the cost of duplicating thunks more frequently, nor do we need the user/library to insert explicit tokens indicating where to duplicate.

(There is no fundamental reason why the dupIO frequency should be tied to the rate at which the buffer is filled, but it seems like an easy way to implement occasional calls to dupIO. I expect that in most cases the rate of allocating Tokens constructors should be proportional to the rate at which we run out of space for a new encoded tokens in the buffer.)

EDIT: this now calls dupIO before evaluating each token in the stream. Testing the above approach on the client's codebase indicated that calling dupIO only when the buffer is full was not enough to avoid the issue. I've also incorporated a fix to #317, though this doesn't seem to make a massive difference to encoding performance.

This still needs dupIO to be released, of course.

I'm unsure whether this is a good idea in general, since dupIO may incur a performance cost of its own. Perhaps it should be guarded by a flag?

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.

None yet

1 participant