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

AK: Don't put element count next to heap-allocated data in FixedArray #24360

Merged
merged 1 commit into from
May 18, 2024

Conversation

DanShaders
Copy link
Contributor

This not only makes code easier to follow but also makes it faster.

PS: I haven't actually tried to find FixedArray's current users in the codebase but this makes my WIP async inflate 5% faster.

This not only makes code easier to follow but also makes it faster.
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 18, 2024
@LucasChollet
Copy link
Member

Not that I'm in favor of one solution or the other, but that was made on purpose in this commit:
f03f70a

@LucasChollet
Copy link
Member

Do you know why it is faster? Is it from the branch removal in size() and data()?

@DanShaders
Copy link
Contributor Author

DanShaders commented May 18, 2024

Yes, because of that and because it does less indirection with size being inline.

@DanShaders
Copy link
Contributor Author

DanShaders commented May 18, 2024

@awesomekling, do you remember where these empty FixedArrays that you were shrinking are?

@awesomekling
Copy link
Member

@awesomekling, where are these empty FixedArrays that you were shrinking?

Looking at the current uses of FixedArray, I'm gonna say "somewhere in the past". 😅
I don't see anything that would justify making FixedArray more complicated.

@awesomekling awesomekling merged commit be36dbc into SerenityOS:master May 18, 2024
11 checks passed
@DanShaders DanShaders deleted the fixed-array-made-okay branch May 18, 2024 16:31
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 18, 2024
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

3 participants