The Wayback Machine - https://web.archive.org/web/20210907114100/https://github.com/ruby/ruby/pull/4673
Skip to content

/ ruby

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

Jump to bottom

Simple event-based profiling tool using chrome://tracing utility #4673

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Simple event-based profiling tool using chrome://tracing utility #4673

wants to merge 1 commit into from
+827 −0

Conversation

@DarkKowalski
Copy link

@DarkKowalski DarkKowalski commented Jul 23, 2021

Notice: Only has been tested on Linux as it uses unistd.h

Inspired by @ko1

He advised me to use chrome://tracing to profile MRI's C Level behaviors

How to use

Enable it

/* In event_profiling.h */
#define USE_EVENT_PROFILING 1

Setup your configuration or just use the default

/* In main.c */
    ruby_sysinit(&argc, &argv);
    {
        RB_SETUP_EVENT_PROFILING_DEFAULT(); // Or RB_SETUP_EVENT_PROFILING()
        RB_SYSTEM_EVENT_PROFILING_BEGIN();

        RUBY_INIT_STACK;
        ruby_init();
        int ret = ruby_run_node(ruby_options(argc, argv));

        RB_SYSTEM_EVENT_PROFILING_END();
        RB_FINALIZE_EVENT_PROFILING_DEFAULT(); // Or RB_FINALIZE_EVENT_PROFILING()

        return ret;
    }

Profile a part of MRI

/* In array.c */
#include "event_profiling.h"
static VALUE
rb_ary_collect(VALUE ary)
{
    RB_EVENT_PROFILING_BEGIN(); // B Event

    long i;
    VALUE collect;
    RETURN_SIZED_ENUMERATOR(ary, 0, 0, ary_enum_length);
    collect = rb_ary_new2(RARRAY_LEN(ary));

   RB_EVENT_PROFILING_SNAPSHOT("Example Snapshot"); // O Event

    for (i = 0; i < RARRAY_LEN(ary); i++) {
        rb_ary_push(collect, rb_yield(RARRAY_AREF(ary, i)));
    }

    RB_EVENT_PROFILING_END(); // E Event

    return collect;
}

Test it (this case is provided by @dsh0416 )

# test.rb
THREADS = 8
LCM = 72072000
t = []

(0...THREADS).each do |i|
  r = Ractor.new i do |j|
    dat = (((LCM/THREADS)*j)...((LCM/THREADS)*(j+1))).to_a
    dat.map{ |a| a ** 2 }.reduce(:+)
  end
  t << r
end

p t.map { |t| t.take }.reduce(:+)

make run will produce a file named event_trace_out.json by default, then open chrome://tracing and load it.

Result:
image
@DarkKowalski DarkKowalski changed the title Simple event-based profiling tool empowered by chrome://tracing utility Simple event-based profiling tool using chrome://tracing utility Jul 23, 2021
@nobu
nobu reviewed Jul 23, 2021
View changes
event_profiling.c Outdated Show resolved Hide resolved
@nobu
nobu reviewed Jul 24, 2021
View changes
event_profiling.h Outdated Show resolved Hide resolved
@ioquatix
Copy link
Member

@ioquatix ioquatix commented Jul 24, 2021

This is pretty cool.

@nobu
nobu requested changes Jul 25, 2021
View changes
main.c Outdated
RB_SYSTEM_EVENT_PROFILING_END();
RB_FINALIZE_EVENT_PROFILING_DEFAULT();

return ret;

This comment has been minimized.

Sign in to view
@nobu

nobu Jul 25, 2021
Member

Do these macros have to be placed in main, not ruby_init, ruby_cleanup and so on?

This comment has been minimized.

Sign in to view
@DarkKowalski

DarkKowalski Jul 25, 2021
Author

I've moved them into ruby.c:ruby_sysinit() and eval.c:rb_ec_cleanup()

common.mk Show resolved Hide resolved
@nobu
nobu requested changes Jul 25, 2021
View changes
Copy link
Member

@nobu nobu left a comment

Why casting away consts instead of making the members const?
event_profiling.c Outdated Show resolved Hide resolved
event_profiling.c Outdated Show resolved Hide resolved
event_profiling.c Outdated Show resolved Hide resolved
event_profiling.c Outdated Show resolved Hide resolved
event_profiling.h Outdated Show resolved Hide resolved
event_profiling.h Outdated Show resolved Hide resolved
@nobu
Copy link
Member

@nobu nobu commented Jul 25, 2021

Could you rebase/squash so that appveyor will run?

@nobu
nobu reviewed Jul 25, 2021
View changes
event_profiling.c Outdated Show resolved Hide resolved
@nobu
Copy link
Member

@nobu nobu commented Jul 25, 2021

Other trivial points:

  • Why casting malloc results?
  • The indentation style differs from the style we use.

@DarkKowalski
Copy link
Author

@DarkKowalski DarkKowalski commented Jul 25, 2021 •
edited

Other trivial points:

  • Why casting malloc results?
  • The indentation style differs from the style we use.

I've removed casting in front of malloc

Do you know any auto format tool (or configurations) I can use to format it?

Thanks.

@DarkKowalski
Copy link
Author

@DarkKowalski DarkKowalski commented Jul 25, 2021

Could you rebase/squash so that appveyor will run?

Sure.

@DarkKowalski DarkKowalski force-pushed the DarkKowalski:kowalski/event_profiling branch 2 times, most recently from 8511307 to bdc157b Jul 25, 2021
@nobu
Copy link
Member

@nobu nobu commented Jul 26, 2021

Do you know any auto format tool (or configurations) I can use to format it?

I'm using misc/ruby-style.el.

Or the following .indent.pro may make sense but GNU indent does too many things than intended.

-bap
-nbbb
-nbc
-br
-nbs
-ncdb
-ce
-cli0.5
-ndj
-nfc1
-i4
-lp
-npcs
-psl
-sc
-sob
-nce
-cdw
-nut
-par
-nsar

-TID
-TVALUE

@DarkKowalski DarkKowalski force-pushed the DarkKowalski:kowalski/event_profiling branch from bdc157b to d77de60 Jul 26, 2021
@DarkKowalski
Copy link
Author

@DarkKowalski DarkKowalski commented Jul 26, 2021

Do you know any auto format tool (or configurations) I can use to format it?

I'm using misc/ruby-style.el.

Or the following .indent.pro may make sense but GNU indent does too many things than intended.

-bap
-nbbb
-nbc
-br
-nbs
-ncdb
-ce
-cli0.5
-ndj
-nfc1
-i4
-lp
-npcs
-psl
-sc
-sob
-nce
-cdw
-nut
-par
-nsar

-TID
-TVALUE

https://www.gnu.org/software/indent/manual/html_node/indent_17.html

There's no option named -nsar. I've formatted my code without -nsar, would you please review again?

@DarkKowalski DarkKowalski requested a review from nobu Jul 28, 2021
@ko1
ko1 reviewed Aug 5, 2021
View changes
event_profiling.h
@@ -0,0 +1,175 @@
#ifndef USE_EVENT_PROFILING

This comment has been minimized.

Sign in to view
@ko1

ko1 Aug 5, 2021
Contributor

This definitions should be in RUBY_EVENT_PROFILING_H guard.

@ko1
ko1 reviewed Aug 5, 2021
View changes
event_profiling.h

#include "ruby/internal/config.h"

#include "vm_core.h"

This comment has been minimized.

Sign in to view
@ko1

ko1 Aug 5, 2021
Contributor

why do you need vm_core.h?

This comment has been minimized.

Sign in to view
@DarkKowalski

DarkKowalski Aug 5, 2021 •
edited
Author

To use GET_RACTOR()

ruby/vm_core.h

Line 1777 in d77de60

#define GET_RACTOR() rb_current_ractor()

in select_profiling_event_list and get_a_profiling_event_slot

@ko1
ko1 reviewed Aug 5, 2021
View changes
ractor.c Outdated
@@ -1584,6 +1585,11 @@ rb_ractor_main_setup(rb_vm_t *vm, rb_ractor_t *r, rb_thread_t *th)
r->pub.self = TypedData_Wrap_Struct(rb_cRactor, &ractor_data_type, r);
FL_SET_RAW(r->pub.self, RUBY_FL_SHAREABLE);
ractor_init(r, Qnil, Qnil);

#if USE_EVENT_PROFILING

This comment has been minimized.

Sign in to view
@ko1

ko1 Aug 5, 2021
Contributor

How about to provide a macro RB_RACTOR_INIT_...

BTW, the subject should be profiling, so RB_EVENT_PROFILING_RACTOR_INIT is one idea.

@ko1
ko1 reviewed Aug 5, 2021
View changes
event_profiling.h Outdated
int max_call_stack_depth;
} rb_event_profiling_config_t;

typedef enum rb_profiling_event_phase

This comment has been minimized.

Sign in to view
@ko1

ko1 Aug 5, 2021
Contributor

The file (component) name is event_profiling, so the prefix can be rb_event_profiling_.

rb_profiling_event_phase_t -> rb_event_profiling_phase_t.

BTW, rb_profiling_event_phase_t is used only once in this patch, so you don't need to make new type.

@ko1
ko1 reviewed Aug 5, 2021
View changes
event_profiling.h Outdated

#else

#define RB_EVENT_PROFILING_BEGIN()

This comment has been minimized.

Sign in to view
@ko1

ko1 Aug 5, 2021
Contributor

How to use them?

@ko1
ko1 reviewed Aug 5, 2021
View changes
event_profiling.c Outdated
#include "ruby/thread_native.h"
#include "vm_core.h"

#if USE_EVENT_PROFILING

This comment has been minimized.

Sign in to view
@ko1

ko1 Aug 5, 2021
Contributor

all above include except event_profiling.h should be in this #if block.

@DarkKowalski DarkKowalski force-pushed the DarkKowalski:kowalski/event_profiling branch from d77de60 to 02fb16e Aug 7, 2021
@DarkKowalski
Improve
f0db22f
@DarkKowalski DarkKowalski force-pushed the DarkKowalski:kowalski/event_profiling branch from 02fb16e to f0db22f Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ko1 ko1

@nobu nobu

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
4 participants
@DarkKowalski @ioquatix @nobu @ko1