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

fix(runtime): Deno.memoryUsage().rss should return correct value #17088

Merged
merged 10 commits into from
Dec 17, 2022

Conversation

dsherret
Copy link
Member

Closes #16581

@dsherret dsherret changed the title fix(runtime): Deno.memoryUsage()#rss should return correct value fix(runtime): Deno.memoryUsage().rss should return correct value Dec 16, 2022
bartlomieju and others added 5 commits December 16, 2022 23:30
…d#17087)

In our `require()` implementation we use a special logic to resolve
"base path" when looking for matching packages, however this logic
is in contradiction to what needs to happen if there's a local
"node_modules"
directory used. This commit changes require implementation to be aware
if we're running off of global node modules cache or a local one.
@bartlomieju
Copy link
Member

@kangelopoulos hopefully this fixes your problem, here's example numbers I get on mac:

$ cargo run
Deno 1.29.1
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> Deno.memoryUsage()
{ rss: 203079680, heapTotal: 3964928, heapUsed: 3556760, external: 225 }
>

std::mem::size_of::<PROCESS_MEMORY_COUNTERS>() as DWORD,
) != FALSE
{
pmc.WorkingSetSize
Copy link
Member Author

Choose a reason for hiding this comment

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

Verified this returns back about the same number as when I do:

> wmic process where processid=<pid goes here> get WorkingSetSize

Copy link
Member

Choose a reason for hiding this comment

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

Verified the same on macos

@magurotuna
Copy link
Member

magurotuna commented Dec 17, 2022

on Ubuntu 22.04.1

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.23s
     Running `target/debug/deno`
Deno 1.29.1
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> Deno.memoryUsage()
{ rss: 217903104, heapTotal: 3915776, heapUsed: 3585416, external: 225 }

Verified the RSS value roughly matches what ps aux | grep deno shows.


For comparison, here's the result of when running on Deno 1.29.1 (released one):

$ deno
Deno 1.29.1
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> Deno.memoryUsage()
{ rss: 3899392, heapTotal: 3907584, heapUsed: 3576700, external: 225 }

@bartlomieju
Copy link
Member

Thanks for checking @magurotuna!

@bartlomieju bartlomieju marked this pull request as ready for review December 17, 2022 14:15
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit e9ecfdd into denoland:main Dec 17, 2022
@bartlomieju bartlomieju deleted the fix_memory_usage_rss branch December 17, 2022 22:25
bartlomieju pushed a commit that referenced this pull request Jan 5, 2023
…17088)

This commit changes implementation of "Deno.memoryUsage()" to return
correct value for "rss" field. To do that we implement a specialized function
per os to retrieve this information.
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.

Deno.memoryUsage Returning rss < heapTotal
3 participants