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

Use cgroups aware processor count by default #969

Closed
wants to merge 1 commit into from
Closed

Use cgroups aware processor count by default #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ PATH
remote: .
specs:
parallel_tests (4.7.1)
concurrent-ruby (~> 1.0, >= 1.3.1)
parallel

GEM
Expand All @@ -17,7 +18,7 @@ GEM
builder (3.2.4)
bump (0.10.0)
colorize (1.1.0)
concurrent-ruby (1.1.10)
concurrent-ruby (1.3.3)
cucumber (4.1.0)
builder (~> 3.2, >= 3.2.3)
cucumber-core (~> 7.1, >= 7.1.0)
Expand Down
3 changes: 2 additions & 1 deletion lib/parallel_tests.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require "parallel"
require "parallel_tests/railtie" if defined? Rails::Railtie
require "rbconfig"
require "concurrent-ruby"

module ParallelTests
WINDOWS = (RbConfig::CONFIG['host_os'] =~ /cygwin|mswin|mingw|bccwin|wince|emx/)
Expand All @@ -17,7 +18,7 @@ def determine_number_of_processes(count)
[
count,
ENV["PARALLEL_TEST_PROCESSORS"],
Parallel.processor_count
Copy link
Owner

Choose a reason for hiding this comment

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

need to remove that method then ?
... best make it call Concurrent.available_processor_count and print a deprecation

Copy link
Author

Choose a reason for hiding this comment

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

Is it OK to remove Parallel.processor_count? I thought we can't remove it because it consider env(PARALLEL_PROCESSOR_COUNT), but Concurrent.available_processor_count not.

Is it better to use Concurrent.available_processor_count inside Parallel.processor_count or add a new method like Parallel.available_processor_count?

Copy link
Owner

Choose a reason for hiding this comment

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

it would be awesome to have Parallel.processor_count use Concurrent.available_processor_count
because otherwise the copy-pasting just continues and afaik these methods are supposed to do the same thing

Copy link
Owner

Choose a reason for hiding this comment

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

possibly even make Parallel.processor_count show a deprecation and tell users to use Concurrent.available_processor_count

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I've created the PR to parallel.
grosser/parallel#348

Concurrent.available_processor_count
].detect { |c| !c.to_s.strip.empty? }.to_i
end

Expand Down
1 change: 1 addition & 0 deletions parallel_tests.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ Gem::Specification.new name, ParallelTests::VERSION do |s|
s.license = "MIT"
s.executables = ["parallel_spinach", "parallel_cucumber", "parallel_rspec", "parallel_test"]
s.add_runtime_dependency "parallel"
s.add_runtime_dependency "concurrent-ruby", "~> 1.0", ">= 1.3.1"
s.required_ruby_version = '>= 3.0.0'
end
2 changes: 1 addition & 1 deletion spec/parallel_tests_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
describe ParallelTests do
describe ".determine_number_of_processes" do
before do
allow(Parallel).to receive(:processor_count).and_return 20
allow(Concurrent).to receive(:available_processor_count).and_return 20
end

def call(count)
Expand Down