Bug #1152

profiler.rb is not concurrent-execution threadsafe

Added by Charles Nutter over 6 years ago. Updated over 2 years ago.

[ruby-core:22046]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:all versions 1.8.6 and up Backport:

Description

=begin
The library profiler.rb uses class variables to store data without wrapping them in mutexes. On current C Ruby, this may only rarely cause a problem, but as more and more code is allowed to run in parallel it's going to lead to issues. If I or other JRuby community members have a chance, we'll try to make it thread-safe, but I wanted to file this issue to ensure it's out there.

The library is also largely unchanged in 1.9.1.

See http://jira.codehaus.org/browse/JRUBY-2133 for the (admittedly sparse) JRuby issue.
=end

1152.patch Magnifier - Sample patch for discussion (2.13 KB) James Abley, 03/16/2009 09:29 AM

1152-2009-03-09.patch Magnifier - latest patch (3.58 KB) James Abley, 03/16/2009 09:37 AM

Associated revisions

Revision 38576
Added by Nobuyoshi Nakada over 2 years ago

profiler.rb: concurrent-execution

  • lib/profiler.rb (Profiler__::PROFILE_PROC, print_profile): store profile data per threads for concurrent-execution. [Bug #1152]

Revision 38576
Added by Nobuyoshi Nakada over 2 years ago

profiler.rb: concurrent-execution

  • lib/profiler.rb (Profiler__::PROFILE_PROC, print_profile): store profile data per threads for concurrent-execution. [Bug #1152]

History

#1 Updated by James Abley about 6 years ago

=begin
Sample patch attached for discussion. There are style and best
practice issues with this patch due to my lack of ruby experience.
Please forgive and be so kind as to suggest improvements.

It is intended to highlight some of the areas that must be considered
when making the change to provide thread-safe profiling.

The patch alters the code to make the capture of the data thread-safe
AFAICT. There may be some issues with the data fields / calculations.
I wasn't completely clear what each field in the array was supposed to
be storing. The main issue unresolved by this patch is how best to
present the information.

I've opted for showing information per-thread and a summary as well.

  • Should there be a version number outputted, in case downstream tools are doing something with this information?
  • Any comments on how the information is being presented??
  • Should profiling be configurable, so that it may not show output per thread, etc.

=end

#2 Updated by James Abley about 6 years ago

=begin
Whoops. Wrong patch attached. Updated version.
=end

#3 Updated by Akira Tanaka almost 4 years ago

  • Project changed from Ruby to Ruby trunk

#4 Updated by Yui NARUSE almost 4 years ago

  • Assignee set to Shugo Maeda
  • Status changed from Open to Assigned

#5 Updated by Shugo Maeda almost 4 years ago

  • Assignee changed from Shugo Maeda to Yukihiro Matsumoto

I'm not the maintainer of profiler.rb. According to git blame,
it seems to be written by Matz and ocean, so I assign this ticket
to Matz.

#6 Updated by Koichi Sasada almost 3 years ago

  • Assignee changed from Yukihiro Matsumoto to Koichi Sasada

I take it.

#7 Updated by Koichi Sasada over 2 years ago

  • Category set to lib
  • Target version set to 2.0.0

#8 Updated by Koichi Sasada over 2 years ago

  • Assignee changed from Koichi Sasada to Nobuyoshi Nakada

nobu, could you check it?

#9 Updated by Nobuyoshi Nakada over 2 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r38576.
Charles, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


profiler.rb: concurrent-execution

  • lib/profiler.rb (Profiler__::PROFILE_PROC, print_profile): store profile data per threads for concurrent-execution. [Bug #1152]

Also available in: Atom PDF