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.

Sign up for GitHub

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

Tentative fix for issue #1634 #1636

Merged
merged 4 commits into from
Dec 11, 2018
Merged

Tentative fix for issue #1634 #1636

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de13f86
Use lsof to get the number of opened files (macOS).
xclerc Dec 11, 2018
6b50579
Add a change entry.
xclerc Dec 11, 2018
69c9621
Add a note explaining why Process is not used.
xclerc Dec 11, 2018
c85ca9e
Merge branch 'master' into macos-stats
xclerc Dec 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/stats.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,28 @@ let enabled = ref false
module Fd_count = struct
type t = Unknown | This of int

let try_to_use_lsof () =
let temp = Filename.temp_file "dune" ".lsof" in
let stdout =
Unix.openfile temp [O_WRONLY; O_CREAT; O_TRUNC; O_SHARE_DELETE] 0o666
in
let prog = "/usr/sbin/lsof" in
let argv = [prog; "-w"; "-p"; string_of_int (Unix.getpid ())] in
let pid = Spawn.spawn ~prog ~argv ~stdout () in
Unix.close stdout;
match Unix.waitpid [] pid with
| _, Unix.WEXITED 0 ->
let num_lines = List.length (Io.input_lines (open_in temp)) in
This (num_lines - 1) (* the output contains a header line *)
| _ -> Unknown
Copy link
Member

Choose a reason for hiding this comment

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

Why not use Process.run_capture (or one of its variations)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was my first move but, if I remember correctly, it unfortunately
created a circular dependency.

Copy link
Member

Choose a reason for hiding this comment

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

I see, that makes sense. Process needs scheduling and scheduling records the stats. I suppose that we'd need a version of Process that doesn't respect the scheduler for it to be usable here. Maybe leave a comment why can't we use Process.run here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, comment added.


let get () =
match Sys.readdir "/proc/self/fd" with
| exception _ -> Unknown
| exception _ ->
begin match try_to_use_lsof () with
| exception _ -> Unknown
| value -> value
end
| files -> This (Array.length files - 1 (* -1 for the dirfd *))

let map2 ~f a b =
Expand Down