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

ArgumentError with sys-proctable 1.2.0 on Windows #24

Closed
andrewngo opened this issue Apr 9, 2018 · 3 comments · Fixed by #36
Closed

ArgumentError with sys-proctable 1.2.0 on Windows #24

andrewngo opened this issue Apr 9, 2018 · 3 comments · Fixed by #36

Comments

@andrewngo
Copy link

Looks like an update to sys-proctable is not backwards compatible with get_process_mem 0.2.1.

System specs:

Windows 10
ruby 2.3.3p222 (2016-11-21 revision 56859) [x64-mingw32]
Using get_process_mem 0.2.1
Using sys-proctable 1.2.0

Repro:

GetProcessMem.new.mb

ArgumentError: wrong number of arguments (given 1, expected 0)

"C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/sys-proctable-1.2.0/lib/windows/sys/proctable.rb:95:in `ps'"
"C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/get_process_mem-0.2.1/lib/get_process_mem.rb:90:in `ps_memory'"
"C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/get_process_mem-0.2.1/lib/get_process_mem.rb:39:in `bytes'"
"C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/get_process_mem-0.2.1/lib/get_process_mem.rb:46:in `mb'"

Workaround:
gem 'sys-proctable', '1.1.5'

@schneems
Copy link
Member

Can you write me a failing test by chance, ideally one that fails on multiple platforms? It still works with mac, so I don't know what to fix:

$ bundle list
Gems included by the bundle:
  * bundler (1.16.1)
  * ffi (1.9.23)
  * get_process_mem (0.2.2)
  * power_assert (1.1.1)
  * rake (10.5.0)
  * sys-proctable (1.2.0)
  * test-unit (3.1.9)
⛄  2.5.1 🚀  ~/documents/projects/get_process_mem (master)
$ be rake test
Loaded suite /Users/rschneeman/.gem/ruby/2.5.1/gems/rake-10.5.0/lib/rake/rake_test_loader
Started
....

Finished in 0.016998 seconds.
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
4 tests, 18 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
235.32 tests/s, 1058.95 assertions/s

@schneems
Copy link
Member

Also this might be a better issue for the sys proctable gem

@andrewngo
Copy link
Author

andrewngo commented Aug 2, 2018

My gemfile

source 'https://rubygems.org'

gem 'get_process_mem'
gem 'sys-proctable', '1.2.1'
bundle list
Gems included by the bundle:
  * bundler (1.16.2)
  * ffi (1.9.25)
  * get_process_mem (0.2.2)
  * sys-proctable (1.2.1)

It looks like sys-proctable is only used on Windows. https://github.com/schneems/get_process_mem/blob/master/lib/get_process_mem.rb#L90

Repro

require 'get_process_mem'

puts Gem.loaded_specs['sys-proctable'].version
puts "Current memory usage (mb): #{GetProcessMem.new.mb}"

They made a breaking change: djberg96/sys-proctable@a0d834a#diff-6eed097efe922533c5d33dc1632d41b2R95

== 1.2.0 - 20-Feb-2018
* There has been an API change. The ProcTable.ps method now uses keyword
  arguments. The 'pid' option is universal, the rest depend on the platform.
  As part of this change, support for Ruby < 2.0 has been dropped.

Would a fix that checks the version work?

  def ps_memory
    if RUNS_ON_WINDOWS
      args = Gem.loaded_specs['sys-proctable'].version > Gem::Version.new('1.1.5') ? {pid: pid} : pid 
      size = ProcTable.ps(args).working_set_size
      BigDecimal.new(size)
    else
      mem = `ps -o rss= -p #{pid}`
      KB_TO_BYTE * BigDecimal.new(mem == "" ? 0 : mem)
    end
  end
bundle exec ruby test.rb
sys-proctable 1.1.5
Current memory usage (mb): 25.9375

bundle exec ruby test.rb
sys-proctable 1.2.1
Current memory usage (mb): 25.88671875

Edit: Oh. Could just fix it, update version and required dependencies. That's better than coding versions in. Duh.

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 a pull request may close this issue.

2 participants