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

feat(compiler)!: Decrease initial memory pages to 1 #2097

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AjaySDwivedi1
Copy link

Changed initial memory pages value to 1

@AjaySDwivedi1
Copy link
Author

I was having some issues downloading and running node.js so if I need to fix or change anything let me know.

@AjaySDwivedi1
Copy link
Author

Can the checkpoints that are failing be explained a little to me? I'm a little lost on what to fix or change.

@ospencer ospencer changed the title Decrease initial memory pages to 1 feat(compiler)!: Decrease initial memory pages to 1 Apr 22, 2024
@ospencer
Copy link
Member

These are two great test failures—they show a case I hadn't considered. We have a flag, --memory-base, which allows you to reserve an amount of memory that the Grain code won't touch. Those two tests request ~1mb of memory be reserved. The error we see is happening because Grain code is trying to write to memory addresses that don't exist.

We'll either need to automatically start with more pages or provide an error to the user that they need to also ask for more pages.

Looking into this I realized there's also a potential compiler bug that could happen from a particularly large program. That's a little involved to fix, so we'll send a PR to handle that and then we can rebase this PR once that fix is in.

Lots of fun from just a two character change 😄

@spotandjake
Copy link
Member

These are two great test failures—they show a case I hadn't considered. We have a flag, --memory-base, which allows you to reserve an amount of memory that the Grain code won't touch. Those two tests request ~1mb of memory be reserved. The error we see is happening because Grain code is trying to write to memory addresses that don't exist.

We'll either need to automatically start with more pages or provide an error to the user that they need to also ask for more pages.

Looking into this I realized there's also a potential compiler bug that could happen from a particularly large program. That's a little involved to fix, so we'll send a PR to handle that and then we can rebase this PR once that fix is in.

Lots of fun from just a two character change 😄

I think we should warn the user that the minimum memory page count is below the requested starting amount, rather than just automagically fixing it. I think in environments where you really care about memory usage it's important that the flags are doing what you say.

@ospencer
Copy link
Member

Yup, that's going to be a part of the fix. Either a warning or an error.

@ospencer
Copy link
Member

Specifically, I wouldn't say we're "fixing" it if the program requires more than one starting page and the user didn't specify a specific number of starting pages. In the cases where they do specify the number of pages, we'll either error/warn.

@JeysonFlores
Copy link

#2095

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