Improving code...

[Joe Van Dyk]

It should be fairly obvious as to what I'm doing (getting information
about a process -- right now just cpu percentage).

Any ideas on how I can improve this code? I'm not a big fan of how
the scanf stuff works.

This is also heavily *nix-biased, and I'm not sure how to properly
work with Windows.



require 'scanf'

class ProcessInfo

def initialize pid
@pid =3D pid
end

def cpu_percentage
# if we don't know the previous time, return 0.0
if @previous_cpu_time.nil? || @previous_time.nil?
@previous_cpu_time =3D get_process_cpu_time
@previous_time =3D Time.now
0.0
else
# Get current and elapsed time
current_time =3D Time.now
elapsed_time =3D current_time - @previous_time

# Get current and elapsed cpu time
current_cpu_time =3D get_process_cpu_time
elapsed_cpu_time =3D current_cpu_time - @previous_cpu_time

# Calculate the percentage used
percentage =3D elapsed_cpu_time / elapsed_time

# Save the current time
@previous_time =3D current_time
@previous_cpu_time =3D current_cpu_time

percentage
end
end

private
# Need to get the process stats
def get_process_cpu_time
stats =3D get_process_stats
stats[:system_cpu_time] + stats[:user_cpu_time]
end

# See 'man proc' for details.
def get_process_stats
proc_stats =3D File.read("/proc/#{ @pid }/stat")

# EEWWWW -- gets the stuff from /proc/<pid>/stat and puts it into vars
pid, comm, state, ppid, pgrp, session, tty_nr, tpgid, flags,
minflt, cminflt, majflt, cmajflt, utime, stime, cutime,
cstime, priority, nice, who_cares, itrealvade,
starttime, vsize, rss, rlim, startcode, endcode, startstack,
kstkesp, kstkeip, signal, blocked, sigignore, sigcatch,
wchan, nswap, cnswap, exit_signal, processor =3D
proc_stats.scanf("%d %s %c %d %d %d %d %d %d %d \
%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d \
%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d")

# Just put the ones I need into the hash
stats =3D {}
stats[:system_cpu_time] =3D stime
stats[:user_cpu_time] =3D utime
stats[rocessor] =3D processor
stats
end
end


# example usage

fail "hey, specify a process id please!" if ARGV.size !=3D 1
pid =3D ARGV.shift
c =3D ProcessInfo.new pid

loop do
puts "current cpu percentage =3D #{ c.cpu_percentage }"
sleep 3
end

 

[Gregory Brown]

def cpu_percentage
# if we don't know the previous time, return 0.0
if @previous_cpu_time.nil? || @previous_time.nil?
@previous_cpu_time =3D get_process_cpu_time
@previous_time =3D Time.now

@previus_cpu_time ||=3D get_process_cpu_time
@previous_time ||=3D Time.now

That drops the conditional.

 

[Gregory Brown]

# See 'man proc' for details.
def get_process_stats
proc_stats =3D File.read("/proc/#{ @pid }/stat")

# EEWWWW -- gets the stuff from /proc/<pid>/stat and puts it into var= s
pid, comm, state, ppid, pgrp, session, tty_nr, tpgid, flags,
minflt, cminflt, majflt, cmajflt, utime, stime, cutime,
cstime, priority, nice, who_cares, itrealvade,
starttime, vsize, rss, rlim, startcode, endcode, startstack,
kstkesp, kstkeip, signal, blocked, sigignore, sigcatch,
wchan, nswap, cnswap, exit_signal, processor =3D
proc_stats.scanf("%d %s %c %d %d %d %d %d %d %d \
%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d \
%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d")

proc_stats =3D File.read("/proc/#{ @pid }/stat").split(" ")

you might need to change your split to match the appropriate pattern,
matching a space above. This makes proc_stats an array

# Just put the ones I need into the hash
stats =3D {}
stats[:system_cpu_time] =3D stime
stats[:user_cpu_time] =3D utime
stats[rocessor] =3D processor
stats
end
end

stats =3D { :system_cpu_time =3D> proc_stats[14],
:user_cpu_time =3D> proc_stats[13],
rocessor =3D> proc_stats.last }

the indicies might not be correct above, but hopefully the idea works

 

[Joe Van Dyk]

# See 'man proc' for details.
def get_process_stats
proc_stats =3D File.read("/proc/#{ @pid }/stat")

# EEWWWW -- gets the stuff from /proc/<pid>/stat and puts it into v= ars
pid, comm, state, ppid, pgrp, session, tty_nr, tpgid, flags,
minflt, cminflt, majflt, cmajflt, utime, stime, cutime,
cstime, priority, nice, who_cares, itrealvade,
starttime, vsize, rss, rlim, startcode, endcode, startstack,
kstkesp, kstkeip, signal, blocked, sigignore, sigcatch,
wchan, nswap, cnswap, exit_signal, processor =3D
proc_stats.scanf("%d %s %c %d %d %d %d %d %d %d \
%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d \
%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d")

proc_stats =3D File.read("/proc/#{ @pid }/stat").split(" ")

you might need to change your split to match the appropriate pattern,
matching a space above. This makes proc_stats an array

# Just put the ones I need into the hash
stats =3D {}
stats[:system_cpu_time] =3D stime
stats[:user_cpu_time] =3D utime
stats[rocessor] =3D processor
stats
end
end

stats =3D { :system_cpu_time =3D> proc_stats[14],
:user_cpu_time =3D> proc_stats[13],
rocessor =3D> proc_stats.last }

the indicies might not be correct above, but hopefully the idea works

That is *exactly* what I had before I decided to use scanf. scanf has
the advantage of automatically converting everything to the correct
type. But I'm not sure what's best in this case.

Later on, I might need to get more of the process information, so I
thought getting all the values, and then just putting the ones I need
into the hash would be better.

Which version would people rather work with?

 

[Joe Van Dyk]

@previus_cpu_time ||=3D get_process_cpu_time
@previous_time ||=3D Time.now

That drops the conditional.

That doesn't return 0.0 if the program doesn't know @previous_cpu_time
or @previous time.

I guess I could've done:
return 0.0 if @previous_cpu_time.nil? or @previous_time.nil?
@previous_cpu_time ||=3D get_process_cpu_time
@previous_time ||=3D Time.now

 

[Gregory Brown]

That is *exactly* what I had before I decided to use scanf. scanf has
the advantage of automatically converting everything to the correct
type. But I'm not sure what's best in this case.

You could easily call to_f before you assign it to the hash.

Later on, I might need to get more of the process information, so I
thought getting all the values, and then just putting the ones I need
into the hash would be better.

you still are getting all the values. You could make an array of keys
or a temporary hash if you want to make it more clear, though...

 

[Gregory Brown]

I guess I could've done:
return 0.0 if @previous_cpu_time.nil? or @previous_time.nil?
@previous_cpu_time ||=3D get_process_cpu_time
@previous_time ||=3D Time.now

Yes, I like that.

 

[Robert Klemme]

It should be fairly obvious as to what I'm doing (getting information
about a process -- right now just cpu percentage).

Any ideas on how I can improve this code? I'm not a big fan of how
the scanf stuff works.

This is also heavily *nix-biased, and I'm not sure how to properly
work with Windows.

Did you consider using Benchmark? I don't know what you're up to
eventually but it might well do what you are looking for.

Kind regards

robert

 

[David A. Black]

Hi --

It should be fairly obvious as to what I'm doing (getting information
about a process -- right now just cpu percentage).

Any ideas on how I can improve this code? I'm not a big fan of how
the scanf stuff works.

Although I feel a parental fondness for scanf.rb, I have to say in
this case you'd probably be better off doing a split and doing the
conversions explicitly. Or you could split, join the ones you want
into a string, and then scanf that.


David

 

[J. Merrill]

joevandyk wrote: (in part)

else
# Get current and elapsed time
current_time = Time.now
elapsed_time = current_time - @previous_time

# Get current and elapsed cpu time
current_cpu_time = get_process_cpu_time
elapsed_cpu_time = current_cpu_time - @previous_cpu_time

Do you want the elapsed cpu time to include the time it takes to compute
the elapsed time? You can rearrange it just a bit --
current_time = Time.now
current_cpu_time = get_process_cpu_time
elapsed_time = current_time - @previous_time
elapsed_cpu_time = current_cpu_time - @previous_cpu_time

-- but that's definitely a nit!