Improving code...

Discussion in 'Ruby' started by Joe Van Dyk, Nov 17, 2005.

  1. Joe Van Dyk

    Joe Van Dyk Guest

    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[:processor] =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
     
    Joe Van Dyk, Nov 17, 2005
    #1
    1. Advertising

  2. On 11/16/05, Joe Van Dyk <> 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 =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, Nov 17, 2005
    #2
    1. Advertising

  3. On 11/16/05, Joe Van Dyk <> wrote:

    > # 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[:processor] =3D processor
    > stats
    > end
    > end


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

    the indicies might not be correct above, but hopefully the idea works ;)
     
    Gregory Brown, Nov 17, 2005
    #3
  4. Joe Van Dyk

    Joe Van Dyk Guest

    On 11/16/05, Gregory Brown <> wrote:
    > On 11/16/05, Joe Van Dyk <> wrote:
    >
    > > # 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[:processor] =3D processor
    > > stats
    > > end
    > > end

    >
    > stats =3D { :system_cpu_time =3D> proc_stats[14],
    > :user_cpu_time =3D> proc_stats[13],
    > :processor =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, Nov 17, 2005
    #4
  5. Joe Van Dyk

    Joe Van Dyk Guest

    On 11/16/05, Gregory Brown <> wrote:
    > On 11/16/05, Joe Van Dyk <> 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 =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.


    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
     
    Joe Van Dyk, Nov 17, 2005
    #5
  6. On 11/17/05, Joe Van Dyk <> 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...
     
    Gregory Brown, Nov 17, 2005
    #6
  7. On 11/17/05, Joe Van Dyk <> wrote:

    > 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.
     
    Gregory Brown, Nov 17, 2005
    #7
  8. 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
     
    Robert Klemme, Nov 17, 2005
    #8
  9. 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

    --
    David A. Black
     
    David A. Black, Nov 17, 2005
    #9
  10. Joe Van Dyk

    J. Merrill Guest

    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!

    --
    Posted via http://www.ruby-forum.com/.
     
    J. Merrill, Nov 17, 2005
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. mysticlol

    improving code

    mysticlol, Sep 15, 2006, in forum: VHDL
    Replies:
    13
    Views:
    824
    mysticlol
    Sep 19, 2006
  2. ruds
    Replies:
    13
    Views:
    597
    squirrel
    Apr 8, 2007
  3. David Lees
    Replies:
    2
    Views:
    398
    Robert Kern
    Feb 25, 2008
  4. mo reina
    Replies:
    5
    Views:
    245
    John Bokma
    Jul 27, 2010
  5. Aditya Mahajan

    Suggestions for improving code

    Aditya Mahajan, Mar 11, 2006, in forum: Ruby
    Replies:
    2
    Views:
    118
    Aditya Mahajan
    Mar 14, 2006
Loading...

Share This Page