Skip to content

Fix page size for aarch64-apple-darwin in test#167

Merged
lhecker merged 1 commit into
microsoft:mainfrom
youknowone:page-size
May 22, 2025
Merged

Fix page size for aarch64-apple-darwin in test#167
lhecker merged 1 commit into
microsoft:mainfrom
youknowone:page-size

Conversation

@youknowone

Copy link
Copy Markdown
Contributor

Page size is not always 4k

@lhecker lhecker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right solution. IMO either we should hardcode it to 16Ki (or 64Ki or more) because the test doesn't actually care about the page size, or we should add a function to sys that returns the OS page size.

What do you think?

e.g. aarch64-apple-darwin has 16KiB page size
@youknowone

Copy link
Copy Markdown
Contributor Author

Because the value is only used in test, I selected hard-coding big value.

page_size crate is used by many crates for this purpose, but this project seems to keep the dependency as small as possible.

@lhecker lhecker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lhecker lhecker merged commit c031f22 into microsoft:main May 22, 2025
1 check passed
@youknowone youknowone deleted the page-size branch May 22, 2025 00:48
@lhecker

lhecker commented May 22, 2025

Copy link
Copy Markdown
Member

[...] but this project seems to keep the dependency as small as possible.

There are various reasons for this, but as I mentioned elsewhere, I'm trying to keep the editor fast and small. Dependencies don't always compose well. Someone asked for instance why I don't use the memchr crate, and that's because our memchr is faster for our specific use case. Someone else asked why I don't use bumpalo, and that's because this wouldn't tightly integrate with the scratch_arena pattern that I like. unicode-rs is problematic because it's too large. Same for regex. And so on...

For page_size however, I'd not use it because this project gets deployed to almost a billion devices worldwide and I don't want to take any risks using dependencies that aren't extraordinarily reputable.

Personally, I also like it because I can learn new tricks this way. What better way is there to grow as an engineer than to challenge oneself?

@youknowone

Copy link
Copy Markdown
Contributor Author

In this case, page_size can be added to dev-dependency. Then it will be run only in test but never will be deployed to users.

Kyza pushed a commit to Kyza/edit that referenced this pull request May 22, 2025
diabloproject pushed a commit to diabloproject/edit that referenced this pull request May 29, 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.

2 participants