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

reduce memory use of pbchunk conversion #820

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Conversation

mflatt
Copy link
Contributor

@mflatt mflatt commented Mar 24, 2024

C code generated by the pbchunk conversion is especially verbose, with long macro names for simple operations and lots of comments to reflect source instructions. Generation of that code was formerly rendering to text in memory, and then splitting into the output files, which turns out to use a lot of memory. Converting to text at the last minute can have a much lower peak memory use.

If memory is at a premimum, then rendering to a temporary file would work even better, but this change already brings memory use in line with other build actions without having to deal with a temporary file. Memory use could also be reduced by dropping comments that connect to source instructions; again, though, doesn't seem necessary for now.

Related to racket/racket#4955.

s/pbchunk.ss Outdated Show resolved Hide resolved
C code generated by the pbchunk conversion is especially verbose, with
long macro names for simple operations and lots of comments to reflect
source instructions. Generation of that code was formerly rendering to
text in memory, and then splitting into the output files, which turns
out to use a lot of memory. Converting to text at the last minute can
have a much lower peak memory use.

If memory is at a premimum, then rendering to a temporary file would
work even better, but this change already brings memory use in line
with other build actions without having to deal with a temporary file.
Memory use could also be reduced by dropping comments that connect to
source instructions; again, though, doesn't seem necessary for now.
@mflatt mflatt merged commit dbce392 into cisco:main Mar 26, 2024
14 of 15 checks 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.

2 participants