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

cpu_time and allocations are 0 when JRuby is used #33471

Merged
merged 1 commit into from Jul 30, 2018

Conversation

yahonda
Copy link
Member

@yahonda yahonda commented Jul 30, 2018

Summary

According to #33449 and #33468, cpu_time and allocations are 0 when JRuby is used.

This pull request addresses the following failure with JRuby.

$ ruby -v
jruby 9.2.1.0-SNAPSHOT (2.5.0) 2018-07-27 13b2df5 Java HotSpot(TM) 64-Bit Server VM 25.181-b13 on 1.8.0_181-b13 [linux-x86_64]
$ bundle exec ruby -w -Itest test/log_subscriber_test.rb -n test_event_attributes
Run options: -n test_event_attributes --seed 6231

F

Failure:
SyncLogSubscriberTest#test_event_attributes [test/log_subscriber_test.rb:84]:
Expected 0 to be > 0.

rails test test/log_subscriber_test.rb:78

Finished in 0.018983s, 52.6791 runs/s, 105.3582 assertions/s.
1 runs, 2 assertions, 1 failures, 0 errors, 0 skips

@rails-bot
Copy link

r? @kaspth

(@rails-bot has picked a reviewer for you, use r? to override)

Copy link
Contributor

@bdewater bdewater left a comment

Choose a reason for hiding this comment

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

This looks like it might silently break on MRI. Perhaps wrap these assertions in a conditional for JRuby?

@yahonda
Copy link
Member Author

yahonda commented Jul 30, 2018

Thanks for the review. Let me update the test case to handle if ruby is JRuby or CRuby.

@@ -80,10 +80,15 @@ def test_event_attributes
instrument "some_event.my_log_subscriber"
wait
event = @log_subscriber.event
if defined?(JRUBY_VERSION)
assert_operator event.cpu_time, :>=, 0
assert_operator event.allocations, :>=, 0
Copy link
Member

Choose a reason for hiding this comment

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

cpu_time and allocations always return 0, so assert_equal 0, ... is preferable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review and useful information. Updated and squashed.

According to rails#33449 and rails#33468, cpu_time and allocations are 0 when
JRuby is used.

```ruby
$ ruby -v
jruby 9.2.1.0-SNAPSHOT (2.5.0) 2018-07-27 13b2df5 Java HotSpot(TM) 64-Bit Server VM 25.181-b13 on 1.8.0_181-b13 [linux-x86_64]
$ bundle exec ruby -w -Itest test/log_subscriber_test.rb -n test_event_attributes
Run options: -n test_event_attributes --seed 6231

F

Failure:
SyncLogSubscriberTest#test_event_attributes [test/log_subscriber_test.rb:84]:
Expected 0 to be > 0.

rails test test/log_subscriber_test.rb:78

Finished in 0.018983s, 52.6791 runs/s, 105.3582 assertions/s.
1 runs, 2 assertions, 1 failures, 0 errors, 0 skips
```
@kamipo kamipo merged commit 159e58f into rails:master Jul 30, 2018
@ahorek
Copy link

ahorek commented Jul 30, 2018

if CLOCK_PROCESS_CPUTIME_ID is implemented on jruby some day, the test will fail. @kampio ?

Correct me if I'm wrong and it's just some known restriction that can never be implemented on JVM

@kamipo
Copy link
Member

kamipo commented Jul 30, 2018

I'm not sure about the restriction, but if CLOCK_PROCESS_CPUTIME_ID is implemented on jruby some day, I think we can just revert the #33468.

@yahonda yahonda deleted the follows_up_33449 branch August 27, 2018 03:22
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 this pull request may close these issues.

None yet

6 participants