Forum: Ruby Improving code...

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
38a8230ed3d5c685558b4f0aad3fc74b?d=identicon&s=25 joevandyk (Guest)
on 2005-11-17 05:39
(Received via mailing list)
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 = 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 = get_process_cpu_time
      @previous_time = Time.now
      0.0
    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

      # Calculate the percentage used
      percentage = elapsed_cpu_time / elapsed_time

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

      percentage
    end
  end

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

  # See 'man proc' for details.
  def get_process_stats
    proc_stats = 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 =
    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 = {}
    stats[:system_cpu_time] = stime
    stats[:user_cpu_time] = utime
    stats[:processor] = processor
    stats
  end
end


 # example usage

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

loop do
  puts "current cpu percentage = #{ c.cpu_percentage }"
  sleep 3
end
31e038e4e9330f6c75ccfd1fca8010ee?d=identicon&s=25 gregory.t.brown (Guest)
on 2005-11-17 05:54
(Received via mailing list)
On 11/16/05, Joe Van Dyk <joevandyk@gmail.com> wrote:

>   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 = get_process_cpu_time
>       @previous_time = Time.now

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

That drops the conditional.
31e038e4e9330f6c75ccfd1fca8010ee?d=identicon&s=25 gregory.t.brown (Guest)
on 2005-11-17 06:03
(Received via mailing list)
On 11/16/05, Joe Van Dyk <joevandyk@gmail.com> wrote:

>       wchan, nswap, cnswap, exit_signal, processor =
>     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 = 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 = {}
>     stats[:system_cpu_time] = stime
>     stats[:user_cpu_time] = utime
>     stats[:processor] = processor
>     stats
>   end
> end

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

the indicies might not be correct above, but hopefully the idea works ;)
38a8230ed3d5c685558b4f0aad3fc74b?d=identicon&s=25 joevandyk (Guest)
on 2005-11-17 06:19
(Received via mailing list)
On 11/16/05, Gregory Brown <gregory.t.brown@gmail.com> wrote:
> >       starttime, vsize, rss, rlim, startcode, endcode, startstack,
>
>                :user_cpu_time     => proc_stats[13],
>                :processor            => 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?
38a8230ed3d5c685558b4f0aad3fc74b?d=identicon&s=25 joevandyk (Guest)
on 2005-11-17 06:22
(Received via mailing list)
On 11/16/05, Gregory Brown <gregory.t.brown@gmail.com> wrote:
>
> 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 ||= get_process_cpu_time
@previous_time ||= Time.now
31e038e4e9330f6c75ccfd1fca8010ee?d=identicon&s=25 gregory.t.brown (Guest)
on 2005-11-17 06:31
(Received via mailing list)
On 11/17/05, Joe Van Dyk <joevandyk@gmail.com> wrote:

> 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...
31e038e4e9330f6c75ccfd1fca8010ee?d=identicon&s=25 gregory.t.brown (Guest)
on 2005-11-17 06:34
(Received via mailing list)
On 11/17/05, Joe Van Dyk <joevandyk@gmail.com> wrote:

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

Yes, I like that.
5befe95e6648daec3dd5728cd36602d0?d=identicon&s=25 bob.news (Guest)
on 2005-11-17 10:20
(Received via mailing list)
Joe Van Dyk wrote:
> 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
1fba4539b6cafe2e60a2916fa184fc2f?d=identicon&s=25 dblack (Guest)
on 2005-11-17 13:48
(Received via mailing list)
Hi --

On Thu, 17 Nov 2005, Joe Van Dyk wrote:

> 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
1d6252c8fa730e6b9989db64df35103a?d=identicon&s=25 J. Merrill (j-merrill)
on 2005-11-18 00:51
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!
This topic is locked and can not be replied to.