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

darwin: reduce memory overhead deleting static member in uv_uptime #4313

Closed
wants to merge 1 commit into from

Conversation

juanarbol
Copy link
Contributor

The uv_uptime function is not the most used function in libuv, storing the uptime selection the whole execution does not make any sense. This patch will make libuv occupy a bit less memory in runtime.

The `uv_uptime` function is not the most used function in libuv, storing
the uptime selection the whole execution does not make any sense. This
patch will make libuv occupy a bit less memory in runtime.

Signed-off-by: Juan José Arboleda <[email protected]>
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

This seems slightly less space efficient now, since it must copy the static values to the stack.

@juanarbol
Copy link
Contributor Author

All the rest of methods are not using a static which, my main concern is not allocation but lifetime. This function called once will result with a copy of the static values for the whole execution, which IMO is not required for a misc function.

@vtjnash
Copy link
Member

vtjnash commented Feb 13, 2024

That does not seem accurate to what static means. The static there means it should not copy the values, but rather use part of the binary to hold the values. Without that, it must generate code to copy those values to the stack.

@juanarbol
Copy link
Contributor Author

@vtjnash Yeah, right; I'm sorry for all the noise here, I'll proceed to close this one.

@juanarbol juanarbol closed this May 22, 2024
@juanarbol juanarbol deleted the juan/remove-static-uptime branch May 22, 2024 05:07
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

2 participants