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

v8: reimplement getHeapStatistics #1824

Closed
wants to merge 1 commit into from

Conversation

vkurchatkin
Copy link
Contributor

Reimplement getHeapStatistics without smalloc as it is going to be removed soon.

R=@trevnorris @bnoordhuis

Reimplement `getHeapStatistics` without `smalloc`
as it is going to be removed soon.
@mscdex mscdex added c++ Issues and PRs that require attention from people who are familiar with C++. v8 engine Issues and PRs related to the V8 dependency. labels May 28, 2015
@bnoordhuis
Copy link
Member

This seems much more convoluted than it needs to be. Why don't you just new Uint32Array(v8binding.kHeapStatisticsBufferLength) in lib/v8.js?

@trevnorris
Copy link
Contributor

How @bnoordhuis suggested is exactly how I was planning on reimplementing it with the new Buffer implementation. But, thanks for taking it off my plate. :)

@vkurchatkin
Copy link
Contributor Author

No problem! I still have to store buffer pointer on env to clean up properly

@bnoordhuis
Copy link
Member

I'm not sure I understand why. If you're worried about externalizing the array, you can use typed_array->Set(index, ...). It's not super fast but you'll only be setting a handful of values.

On second thought, you could even use a regular array. I doubt anyone will notice the difference.

@vkurchatkin
Copy link
Contributor Author

This should be covered by #2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants