JIT debugging (Attach and speed)

JIT debugging (Attach and speed)

[Yichao Yu]

Hi,

I've recently seen some issues when debugging julia JIT code with GDB.
Some of these might worth a bug report but I'd like to post here first
since the most blocking issue was discussed here a few years ago.

1. Registering JIT code to GDB is O(n^2) and this is very bad for
serious JIT users like julia. (I've seen 10k to 100k jit objects
generated in the test)

   The issue was discussed on this list ~2011[1] but it seems that the
issue is still there. I was also told that lldb 3.8 should support the
same JIT debugging interface without the O(n^2) slow down[2].

2. JIT code registration on attach is broken.

    When I set a breakpoint on `jit_inferior_init`[3] (i.e. lauching
gdb with `gdb --args gdb -p <pid_to_debug>`) which IIUC is what
responsible for walking the jit object list at init time, it seems
that the function is never called.

    (I haven't seen a bug report about this yet)

3. (minor) Reading debug info from JIT object file generates read of large size.

   This triggers an assertion in rr[4]. It's arguably rr's fault to
assume a upper limit on the read request size (fixed now) but since
this doesn't happen otherwise I'd just like to point this out in case
some optimization is needed here.

[1] https://sourceware.org/ml/gdb/2011-01/msg00002.html
[2] https://github.com/JuliaLang/julia/issues/14846#issuecomment-176902134
[3] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/jit.c;h=afc1c517fa745582bd198601e46a77af72114017;hb=HEAD#l1323
[4] https://github.com/mozilla/rr/issues/1665#issuecomment-192983671

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/22/2016 02:55 PM, Yichao Yu wrote:
> Hi,
>
> I've recently seen some issues when debugging julia JIT code with GDB.
> Some of these might worth a bug report but I'd like to post here first
> since the most blocking issue was discussed here a few years ago.
>
> 1. Registering JIT code to GDB is O(n^2) and this is very bad for
> serious JIT users like julia. (I've seen 10k to 100k jit objects
> generated in the test)
>
>     The issue was discussed on this list ~2011[1] but it seems that the
> issue is still there. I was also told that lldb 3.8 should support the
> same JIT debugging interface without the O(n^2) slow down[2].

I re-read the 2011 discussion, and it seems like we had an idea for a
fix:

  https://sourceware.org/ml/gdb/2011-01/msg00011.html

Paul, did you ever manage to get that working?

>
> 2. JIT code registration on attach is broken.
>
>      When I set a breakpoint on `jit_inferior_init`[3] (i.e. lauching
> gdb with `gdb --args gdb -p <pid_to_debug>`) which IIUC is what
> responsible for walking the jit object list at init time, it seems
> that the function is never called.
>
>      (I haven't seen a bug report about this yet)

Do you know whether this happens with 7.11 and master, and if so,
would it be possible for you to git bisect the culprit?

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Paul Pluzhnikov]

On Tue, Mar 22, 2016 at 8:46 AM, Pedro Alves <palves@redhat.com> wrote:

> I re-read the 2011 discussion, and it seems like we had an idea for a
> fix:
>
>  https://sourceware.org/ml/gdb/2011-01/msg00011.html
>
> Paul, did you ever manage to get that working?

It's been a long time and I lost all context, but I don't believe I've
ever finished this. Sorry!


-- 
Paul Pluzhnikov

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/22/2016 03:46 PM, Pedro Alves wrote:

>> 2. JIT code registration on attach is broken.
>>
>>      When I set a breakpoint on `jit_inferior_init`[3] (i.e. lauching
>> gdb with `gdb --args gdb -p <pid_to_debug>`) which IIUC is what
>> responsible for walking the jit object list at init time, it seems
>> that the function is never called.
>>
>>      (I haven't seen a bug report about this yet)
>
> Do you know whether this happens with 7.11 and master, and if so,
> would it be possible for you to git bisect the culprit?

Currently, jit_inferior_created_hook -> jit_inferior_init is only
called when the inferior execs...

Grepping around, I think that might have been
the fix for PR gdb/13431 (03bef283c2d3):
  https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
which removed the inferior_created (jit_inferior_created_observer).

Adding an inferior_created observer back likely fixes the issue.

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Yichao Yu]

On Tue, Mar 22, 2016 at 12:15 PM, Pedro Alves <palves@redhat.com> wrote:
> On 03/22/2016 03:46 PM, Pedro Alves wrote:

> I re-read the 2011 discussion, and it seems like we had an idea for a fix:

IIUC the proposed fix might cause regression in some cases?

>>
>> Do you know whether this happens with 7.11 and master, and if so,
>> would it be possible for you to git bisect the culprit?

This is 7.11 package from ArchLinux. I could try bi-secting although
apparently you are faster at pin-point the issue.

>
> Currently, jit_inferior_created_hook -> jit_inferior_init is only
> called when the inferior execs...
>
> Grepping around, I think that might have been
> the fix for PR gdb/13431 (03bef283c2d3):
>  https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
> which removed the inferior_created (jit_inferior_created_observer).
>
> Adding an inferior_created observer back likely fixes the issue.

I'm happy to test patches.

>
> Thanks,
> Pedro Alves
>

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/22/2016 04:22 PM, Yichao Yu wrote:
> On Tue, Mar 22, 2016 at 12:15 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 03/22/2016 03:46 PM, Pedro Alves wrote:
> 
>> I re-read the 2011 discussion, and it seems like we had an idea for a fix:
> 
> IIUC the proposed fix might cause regression in some cases?

Yeah, there's no full fix available, only some ideas thrown out.
The last discussed one wouldn't cause a regression -- the
"longjmp"-caching idea.  We may still need to defer breakpoint re-set
to at most once per jit load event, something like Paul's original
patch, but with a breakpoint_re_set call somewhere.

It'd even be better to somehow restrict breakpoint re-setting
to the jit modules that were added/removed/changed, but
that's harder.

> 
>>>
>>> Do you know whether this happens with 7.11 and master, and if so,
>>> would it be possible for you to git bisect the culprit?
> 
> This is 7.11 package from ArchLinux. I could try bi-secting although
> apparently you are faster at pin-point the issue.
> 
>>
>> Currently, jit_inferior_created_hook -> jit_inferior_init is only
>> called when the inferior execs...
>>
>> Grepping around, I think that might have been
>> the fix for PR gdb/13431 (03bef283c2d3):
>>   https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
>> which removed the inferior_created (jit_inferior_created_observer).
>>
>> Adding an inferior_created observer back likely fixes the issue.
> 
> I'm happy to test patches.

I'm happy to provide guidance, but a fix would likely happen faster
if someone else stepped up to write it.

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Yichao Yu]

On Tue, Mar 22, 2016 at 12:41 PM, Pedro Alves <palves@redhat.com> wrote:
> On 03/22/2016 04:22 PM, Yichao Yu wrote:
>> On Tue, Mar 22, 2016 at 12:15 PM, Pedro Alves <palves@redhat.com> wrote:
>>> On 03/22/2016 03:46 PM, Pedro Alves wrote:
>>
>>> I re-read the 2011 discussion, and it seems like we had an idea for a fix:
>>
>> IIUC the proposed fix might cause regression in some cases?
>
> Yeah, there's no full fix available, only some ideas thrown out.
> The last discussed one wouldn't cause a regression -- the
> "longjmp"-caching idea.  We may still need to defer breakpoint re-set
> to at most once per jit load event, something like Paul's original
> patch, but with a breakpoint_re_set call somewhere.
>
> It'd even be better to somehow restrict breakpoint re-setting
> to the jit modules that were added/removed/changed, but
> that's harder.
>
>>
>>>>
>>>> Do you know whether this happens with 7.11 and master, and if so,
>>>> would it be possible for you to git bisect the culprit?
>>
>> This is 7.11 package from ArchLinux. I could try bi-secting although
>> apparently you are faster at pin-point the issue.
>>
>>>
>>> Currently, jit_inferior_created_hook -> jit_inferior_init is only
>>> called when the inferior execs...
>>>
>>> Grepping around, I think that might have been
>>> the fix for PR gdb/13431 (03bef283c2d3):
>>>   https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
>>> which removed the inferior_created (jit_inferior_created_observer).
>>>
>>> Adding an inferior_created observer back likely fixes the issue.
>>
>> I'm happy to test patches.
>
> I'm happy to provide guidance, but a fix would likely happen faster
> if someone else stepped up to write it.

Are these lines (or at least the first one) the ones you think should
be added back?

-  observer_attach_inferior_created (jit_inferior_created_observer);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
-  observer_attach_executable_changed (jit_executable_changed_observer);

I can try that although I'm not particularly sure what was the reason
they are removed and how to check for regressions.

>
> Thanks,
> Pedro Alves
>

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/22/2016 04:47 PM, Yichao Yu wrote:
> On Tue, Mar 22, 2016 at 12:41 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 03/22/2016 04:22 PM, Yichao Yu wrote:
>>> On Tue, Mar 22, 2016 at 12:15 PM, Pedro Alves <palves@redhat.com> wrote:
>>>> On 03/22/2016 03:46 PM, Pedro Alves wrote:
>>>
>>>> I re-read the 2011 discussion, and it seems like we had an idea for a fix:
>>>
>>> IIUC the proposed fix might cause regression in some cases?
>>
>> Yeah, there's no full fix available, only some ideas thrown out.
>> The last discussed one wouldn't cause a regression -- the
>> "longjmp"-caching idea.  We may still need to defer breakpoint re-set
>> to at most once per jit load event, something like Paul's original
>> patch, but with a breakpoint_re_set call somewhere.
>>
>> It'd even be better to somehow restrict breakpoint re-setting
>> to the jit modules that were added/removed/changed, but
>> that's harder.
>>
>>>
>>>>>
>>>>> Do you know whether this happens with 7.11 and master, and if so,
>>>>> would it be possible for you to git bisect the culprit?
>>>
>>> This is 7.11 package from ArchLinux. I could try bi-secting although
>>> apparently you are faster at pin-point the issue.
>>>
>>>>
>>>> Currently, jit_inferior_created_hook -> jit_inferior_init is only
>>>> called when the inferior execs...
>>>>
>>>> Grepping around, I think that might have been
>>>> the fix for PR gdb/13431 (03bef283c2d3):
>>>>    https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
>>>> which removed the inferior_created (jit_inferior_created_observer).
>>>>
>>>> Adding an inferior_created observer back likely fixes the issue.
>>>
>>> I'm happy to test patches.
>>
>> I'm happy to provide guidance, but a fix would likely happen faster
>> if someone else stepped up to write it.
> 
> Are these lines (or at least the first one) the ones you think should
> be added back?
> 
> -  observer_attach_inferior_created (jit_inferior_created_observer);
>     observer_attach_inferior_exit (jit_inferior_exit_hook);
> -  observer_attach_executable_changed (jit_executable_changed_observer);
> 

Something like that.  At least the first one.  Not sure the second is
needed, since with Tromey's change the data is associated with the objfile.

> I can try that although I'm not particularly sure what was the reason
> they are removed 

Not sure either.  I assume studying Tromey's description of the original
change helps bring that to light.

> and how to check for regressions.

GDB has a regression test suite under src/gdb/testsuite/.  The
gdb/testsuite/README file has instructions.  

Basically, run "make check -j8" before the patch, "make check -j8"
after the patch, and diff the resulting testsuite/gdb.sum files.

Note that there are some tests that may be racy on your machine, so you
may get unrelated some noise.  Running a particular test a
couple times, with:

 make check TESTS="gdb.base/foo.exp"

should help you determine whether that's the case.

It'd be very nice if we had a _new_ test that covers your use case,
to avoid regressing again.  That likely makes the patch bigger than
what we could accept without a copyright assignment though.  If you'd
like to pursue that, let me know and I'll send you the forms.

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Yichao Yu]

Sorry for the delay, I was busy with other stuff...

>>>>> I re-read the 2011 discussion, and it seems like we had an idea for a fix:
>>>>
>>>> IIUC the proposed fix might cause regression in some cases?
>>>
>>> Yeah, there's no full fix available, only some ideas thrown out.
>>> The last discussed one wouldn't cause a regression -- the
>>> "longjmp"-caching idea.  We may still need to defer breakpoint re-set
>>> to at most once per jit load event, something like Paul's original
>>> patch, but with a breakpoint_re_set call somewhere.
>>>
>>> It'd even be better to somehow restrict breakpoint re-setting
>>> to the jit modules that were added/removed/changed, but
>>> that's harder.
>>>
>>>>
>>>>>>
>>>>>> Do you know whether this happens with 7.11 and master, and if so,
>>>>>> would it be possible for you to git bisect the culprit?
>>>>
>>>> This is 7.11 package from ArchLinux. I could try bi-secting although
>>>> apparently you are faster at pin-point the issue.
>>>>
>>>>>
>>>>> Currently, jit_inferior_created_hook -> jit_inferior_init is only
>>>>> called when the inferior execs...
>>>>>
>>>>> Grepping around, I think that might have been
>>>>> the fix for PR gdb/13431 (03bef283c2d3):
>>>>>    https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
>>>>> which removed the inferior_created (jit_inferior_created_observer).
>>>>>
>>>>> Adding an inferior_created observer back likely fixes the issue.
>>>>
>>>> I'm happy to test patches.
>>>
>>> I'm happy to provide guidance, but a fix would likely happen faster
>>> if someone else stepped up to write it.
>>
>> Are these lines (or at least the first one) the ones you think should
>> be added back?
>>
>> -  observer_attach_inferior_created (jit_inferior_created_observer);
>>     observer_attach_inferior_exit (jit_inferior_exit_hook);
>> -  observer_attach_executable_changed (jit_executable_changed_observer);
>>
>
> Something like that.  At least the first one.  Not sure the second is
> needed, since with Tromey's change the data is associated with the objfile.
>
>> I can try that although I'm not particularly sure what was the reason
>> they are removed
>
> Not sure either.  I assume studying Tromey's description of the original
> change helps bring that to light.
>
>> and how to check for regressions.
>
> GDB has a regression test suite under src/gdb/testsuite/.  The
> gdb/testsuite/README file has instructions.
>
> Basically, run "make check -j8" before the patch, "make check -j8"
> after the patch, and diff the resulting testsuite/gdb.sum files.
>
> Note that there are some tests that may be racy on your machine, so you
> may get unrelated some noise.  Running a particular test a
> couple times, with:
>
>  make check TESTS="gdb.base/foo.exp"
>
> should help you determine whether that's the case.
>
> It'd be very nice if we had a _new_ test that covers your use case,
> to avoid regressing again.  That likely makes the patch bigger than
> what we could accept without a copyright assignment though.  If you'd
> like to pursue that, let me know and I'll send you the forms.

I've got a simple patch that fixes the issue for me and AFAICT all of
the failing tests are racy and/or failing on this machine before the
change too. I haven't add test yet since I'm not so sure how to add it
(I found test for both jit interface and attach but haven't figured
out how to write a new one yet...).

I'm happy to complete copyright related forms necessary.

>
> Thanks,
> Pedro Alves
>


From a94b2c68d83e13ee80e5c21ab27dfedaddfda590 Mon Sep 17 00:00:00 2001
From: Yichao Yu <yyc1992@gmail.com>
Date: Tue, 22 Mar 2016 15:24:11 -0400
Subject: [PATCH] Fix JIT debug when attaching to a process.

---
 gdb/jit.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index afc1c51..0bd127b 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1026,7 +1026,7 @@ jit_breakpoint_deleted (struct breakpoint *b)
 }

 /* (Re-)Initialize the jit breakpoint if necessary.
-   Return 0 on success.  */
+   Return 0 if the jit breakpoint has been successfully initialized.  */

 static int
 jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
@@ -1070,7 +1070,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
                        paddress (gdbarch, addr));

   if (ps_data->cached_code_address == addr)
-    return 1;
+    return 0;

   /* Delete the old breakpoint.  */
   if (ps_data->jit_breakpoint != NULL)
@@ -1288,7 +1288,8 @@ static const struct frame_unwind jit_frame_unwind =
   jit_frame_prev_register,
   NULL,
   jit_frame_sniffer,
-  jit_dealloc_cache
+  jit_dealloc_cache,
+  NULL
 };


@@ -1375,6 +1376,12 @@ jit_inferior_created_hook (void)
   jit_inferior_init (target_gdbarch ());
 }

+static void
+jit_inferior_created (struct target_ops *ops, int from_tty)
+{
+  jit_inferior_created_hook ();
+}
+
 /* Exported routine to call to re-set the jit breakpoints,
    e.g. when a program is rerun.  */

@@ -1496,6 +1503,7 @@ _initialize_jit (void)
                             show_jit_debug,
                             &setdebuglist, &showdebuglist);

+  observer_attach_inferior_created (jit_inferior_created);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
   observer_attach_breakpoint_deleted (jit_breakpoint_deleted);

-- 
2.7.4

Re: JIT debugging (Attach and speed)

[Yichao Yu]

>>>> Yeah, there's no full fix available, only some ideas thrown out.
>>>> The last discussed one wouldn't cause a regression -- the
>>>> "longjmp"-caching idea.  We may still need to defer breakpoint re-set
>>>> to at most once per jit load event, something like Paul's original
>>>> patch, but with a breakpoint_re_set call somewhere.

I've just run the profile myself and got some quite different result
from the 2011 thread[1].
With no breakpoint in gdb and simply jitting O(2000) functions: [2]
With one (un-triggered) breakpoint in gdb and jitting O(1500) functions: [3]

It seems that there's other slowness when there are breakpoint created
but in terms of scaling, the fastest growing one is the sorting in
`update_section_map`

[1] https://sourceware.org/ml/gdb/2011-01/msg00009.html
[2] http://i.imgur.com/6Ca6Yal.jpg
[3] http://i.imgur.com/aHKGACX.jpg

>>>> It'd even be better to somehow restrict breakpoint re-setting
>>>> to the jit modules that were added/removed/changed, but
>>>> that's harder.

I've re-read the 2011 thread and I think I have a slightly better
understanding now. IIUC we need to check if the newly registered file
contains any pending breakpoints. Is the problem that this is done by
checking it over all the registered object files? Is it possible to
avoid O(n^2) scaling without only re-setting on the jit object file
currently being modified?

>>>>
>>>>>
>>>>>>>
>>>>>>> Do you know whether this happens with 7.11 and master, and if so,
>>>>>>> would it be possible for you to git bisect the culprit?
>>>>>
>>>>> This is 7.11 package from ArchLinux. I could try bi-secting although
>>>>> apparently you are faster at pin-point the issue.
>>>>>
>>>>>>
>>>>>> Currently, jit_inferior_created_hook -> jit_inferior_init is only
>>>>>> called when the inferior execs...
>>>>>>
>>>>>> Grepping around, I think that might have been
>>>>>> the fix for PR gdb/13431 (03bef283c2d3):
>>>>>>    https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
>>>>>> which removed the inferior_created (jit_inferior_created_observer).
>>>>>>
>>>>>> Adding an inferior_created observer back likely fixes the issue.
>>>>>
>>>>> I'm happy to test patches.
>>>>
>>>> I'm happy to provide guidance, but a fix would likely happen faster
>>>> if someone else stepped up to write it.
>>>
>>> Are these lines (or at least the first one) the ones you think should
>>> be added back?
>>>
>>> -  observer_attach_inferior_created (jit_inferior_created_observer);
>>>     observer_attach_inferior_exit (jit_inferior_exit_hook);
>>> -  observer_attach_executable_changed (jit_executable_changed_observer);
>>>
>>
>> Something like that.  At least the first one.  Not sure the second is
>> needed, since with Tromey's change the data is associated with the objfile.
>>
>>> I can try that although I'm not particularly sure what was the reason
>>> they are removed
>>
>> Not sure either.  I assume studying Tromey's description of the original
>> change helps bring that to light.
>>
>>> and how to check for regressions.
>>
>> GDB has a regression test suite under src/gdb/testsuite/.  The
>> gdb/testsuite/README file has instructions.
>>
>> Basically, run "make check -j8" before the patch, "make check -j8"
>> after the patch, and diff the resulting testsuite/gdb.sum files.
>>
>> Note that there are some tests that may be racy on your machine, so you
>> may get unrelated some noise.  Running a particular test a
>> couple times, with:
>>
>>  make check TESTS="gdb.base/foo.exp"
>>
>> should help you determine whether that's the case.
>>
>> It'd be very nice if we had a _new_ test that covers your use case,
>> to avoid regressing again.  That likely makes the patch bigger than
>> what we could accept without a copyright assignment though.  If you'd
>> like to pursue that, let me know and I'll send you the forms.
>
> I've got a simple patch that fixes the issue for me and AFAICT all of
> the failing tests are racy and/or failing on this machine before the
> change too. I haven't add test yet since I'm not so sure how to add it
> (I found test for both jit interface and attach but haven't figured
> out how to write a new one yet...).
>
> I'm happy to complete copyright related forms necessary.
>
>>
>> Thanks,
>> Pedro Alves
>>
>
>
> From a94b2c68d83e13ee80e5c21ab27dfedaddfda590 Mon Sep 17 00:00:00 2001
> From: Yichao Yu <yyc1992@gmail.com>
> Date: Tue, 22 Mar 2016 15:24:11 -0400
> Subject: [PATCH] Fix JIT debug when attaching to a process.
>
> ---
>  gdb/jit.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/jit.c b/gdb/jit.c
> index afc1c51..0bd127b 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -1026,7 +1026,7 @@ jit_breakpoint_deleted (struct breakpoint *b)
>  }
>
>  /* (Re-)Initialize the jit breakpoint if necessary.
> -   Return 0 on success.  */
> +   Return 0 if the jit breakpoint has been successfully initialized.  */
>
>  static int
>  jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
> @@ -1070,7 +1070,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>                         paddress (gdbarch, addr));
>
>    if (ps_data->cached_code_address == addr)
> -    return 1;
> +    return 0;
>
>    /* Delete the old breakpoint.  */
>    if (ps_data->jit_breakpoint != NULL)
> @@ -1288,7 +1288,8 @@ static const struct frame_unwind jit_frame_unwind =
>    jit_frame_prev_register,
>    NULL,
>    jit_frame_sniffer,
> -  jit_dealloc_cache
> +  jit_dealloc_cache,
> +  NULL
>  };
>
>
> @@ -1375,6 +1376,12 @@ jit_inferior_created_hook (void)
>    jit_inferior_init (target_gdbarch ());
>  }
>
> +static void
> +jit_inferior_created (struct target_ops *ops, int from_tty)
> +{
> +  jit_inferior_created_hook ();
> +}
> +
>  /* Exported routine to call to re-set the jit breakpoints,
>     e.g. when a program is rerun.  */
>
> @@ -1496,6 +1503,7 @@ _initialize_jit (void)
>                              show_jit_debug,
>                              &setdebuglist, &showdebuglist);
>
> +  observer_attach_inferior_created (jit_inferior_created);
>    observer_attach_inferior_exit (jit_inferior_exit_hook);
>    observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
>
> --
> 2.7.4

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/23/2016 02:18 AM, Yichao Yu wrote:

>> It'd be very nice if we had a _new_ test that covers your use case,
>> to avoid regressing again.  That likely makes the patch bigger than
>> what we could accept without a copyright assignment though.  If you'd
>> like to pursue that, let me know and I'll send you the forms.
>
> I've got a simple patch that fixes the issue for me and AFAICT all of
> the failing tests are racy and/or failing on this machine before the
> change too. I haven't add test yet since I'm not so sure how to add it
> (I found test for both jit interface and attach but haven't figured
> out how to write a new one yet...).

Excellent, this helps a lot.

I have an idea for the test, so I'll handle it.  Let me bundle
things in a proper patch and post it to gdb-patches.  Stay tuned.

>
> I'm happy to complete copyright related forms necessary.

I'll send this off list.

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/23/2016 12:24 PM, Pedro Alves wrote:
> On 03/23/2016 02:18 AM, Yichao Yu wrote:
>
>>> It'd be very nice if we had a _new_ test that covers your use case,
>>> to avoid regressing again.  That likely makes the patch bigger than
>>> what we could accept without a copyright assignment though.  If you'd
>>> like to pursue that, let me know and I'll send you the forms.
>>
>> I've got a simple patch that fixes the issue for me and AFAICT all of
>> the failing tests are racy and/or failing on this machine before the
>> change too. I haven't add test yet since I'm not so sure how to add it
>> (I found test for both jit interface and attach but haven't figured
>> out how to write a new one yet...).
>
> Excellent, this helps a lot.
>
> I have an idea for the test, so I'll handle it.  Let me bundle
> things in a proper patch and post it to gdb-patches.  Stay tuned.

Posted:
  https://sourceware.org/ml/gdb-patches/2016-03/msg00465.html

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/23/2016 04:51 AM, Yichao Yu wrote:
>>>>> Yeah, there's no full fix available, only some ideas thrown out.
>>>>> The last discussed one wouldn't cause a regression -- the
>>>>> "longjmp"-caching idea.  We may still need to defer breakpoint re-set
>>>>> to at most once per jit load event, something like Paul's original
>>>>> patch, but with a breakpoint_re_set call somewhere.
>
> I've just run the profile myself and got some quite different result
> from the 2011 thread[1].
> With no breakpoint in gdb and simply jitting O(2000) functions: [2]
> With one (un-triggered) breakpoint in gdb and jitting O(1500) functions: [3]
>
> It seems that there's other slowness when there are breakpoint created
> but in terms of scaling, the fastest growing one is the sorting in
> `update_section_map`
>
> [1] https://sourceware.org/ml/gdb/2011-01/msg00009.html
> [2] http://i.imgur.com/6Ca6Yal.jpg
> [3] http://i.imgur.com/aHKGACX.jpg

Ouch.  Not sure what to do here.

Maybe we could cache something above find_pc_section ->
update_section_map, to avoid the section look ups.
 From [2], we see that this is in the inline frame sniffer.
Maybe we can add a shortcut here, based on assuming that if
we're stopped at the jit event breakpoint address, then we're
not stopped at an inlining site.  That is, a trick similar to
the pc_at_non_inline_function check in infrun.c.

So, as a quick hack, if you make inline_frame_sniffer bail out early
with 0, does it improve things?

>
>>>>> It'd even be better to somehow restrict breakpoint re-setting
>>>>> to the jit modules that were added/removed/changed, but
>>>>> that's harder.
>
> I've re-read the 2011 thread and I think I have a slightly better
> understanding now. IIUC we need to check if the newly registered file
> contains any pending breakpoints. Is the problem that this is done by
> checking it over all the registered object files? Is it possible to
> avoid O(n^2) scaling without only re-setting on the jit object file
> currently being modified?

Batch loading the object files comes to mind.  From reading
the code, it seems like gdb only reads on object file per
JIT_REGISTER event.  The descriptor->relevant_entry one.
Is that correct?  Is there a reason the loader couldn't batch
compile all functions, only call the JIT_REGISTER once, and
then have GDB follow descriptor->relevant_entry->next_entry, etc.?

If we could batch load, then we could also defer breakpoint re-set
until all is loaded, as well as inhibit section map updates,
as done in svr4_handle_solib_event for normal DSO loading.

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Yichao Yu]

>>
>> [1] https://sourceware.org/ml/gdb/2011-01/msg00009.html
>> [2] http://i.imgur.com/6Ca6Yal.jpg
>> [3] http://i.imgur.com/aHKGACX.jpg
>
>
> Ouch.  Not sure what to do here.
>
> Maybe we could cache something above find_pc_section ->
> update_section_map, to avoid the section look ups.
> From [2], we see that this is in the inline frame sniffer.
> Maybe we can add a shortcut here, based on assuming that if
> we're stopped at the jit event breakpoint address, then we're
> not stopped at an inlining site.  That is, a trick similar to
> the pc_at_non_inline_function check in infrun.c.
>
> So, as a quick hack, if you make inline_frame_sniffer bail out early
> with 0, does it improve things?

With

diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index f8ba249..49cae00 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -206,6 +206,7 @@ inline_frame_sniffer (const struct frame_unwind *self,
                      struct frame_info *this_frame,
                      void **this_cache)
 {
+  return 0;
   CORE_ADDR this_pc;
   const struct block *frame_block, *cur_block;
   int depth;

The timing looks the same. =(

>
>>
>>>>>> It'd even be better to somehow restrict breakpoint re-setting
>>>>>> to the jit modules that were added/removed/changed, but
>>>>>> that's harder.
>>
>>
>> I've re-read the 2011 thread and I think I have a slightly better
>> understanding now. IIUC we need to check if the newly registered file
>> contains any pending breakpoints. Is the problem that this is done by
>> checking it over all the registered object files? Is it possible to
>> avoid O(n^2) scaling without only re-setting on the jit object file
>> currently being modified?
>
>
> Batch loading the object files comes to mind.  From reading
> the code, it seems like gdb only reads on object file per
> JIT_REGISTER event.  The descriptor->relevant_entry one.
> Is that correct?  Is there a reason the loader couldn't batch
> compile all functions, only call the JIT_REGISTER once, and
> then have GDB follow descriptor->relevant_entry->next_entry, etc.?

What do you mean by "loader"? since you mentioned call JIT_REGISTER I
assume you mean JIT compiler?

At least for the julia JIT, we already try to batch compile as much as
we can (this is also necessary to save memory for example, since most
functions are much smaller than a page) However, it is impossible to
know all the functions a program is going to call so there'll be
continues jitting going on. For normal program this shouldn't be that
bad since it should execute a small number of functions most of the
time and the jit don't need to work anymore after some initial warm
up. However, the test suite is a very case since it is written to call
all the functions with many different types which will force the
compiler to emit a lot of functions and run them.

>
> If we could batch load, then we could also defer breakpoint re-set
> until all is loaded, as well as inhibit section map updates,
> as done in svr4_handle_solib_event for normal DSO loading.
>
> Thanks,
> Pedro Alves
>

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/23/2016 07:32 PM, Yichao Yu wrote:

>> So, as a quick hack, if you make inline_frame_sniffer bail out early
>> with 0, does it improve things?
> 

> The timing looks the same. =(

Bummer.  :-/

Having now looked at the gdb.base/jit.exp test, I see that
we can easily reproduce this with:

gdb ---args ./testsuite/outputs/gdb.base/jit/jit-main ./testsuite/outputs/gdb.base/jit/jit-solib.so 2000

(It'd be nice to turn this into a check-perf test...)

> What do you mean by "loader"? since you mentioned call JIT_REGISTER I
> assume you mean JIT compiler?

Yes, sorry.  I meant whatever code ends up "sending" a JIT_REGISTER
event to gdb.

Thanks,
Pedro Alves

Re: JIT debugging (Attach and speed)

[Yichao Yu]

On Wed, Mar 23, 2016 at 3:48 PM, Pedro Alves <palves@redhat.com> wrote:
> On 03/23/2016 07:32 PM, Yichao Yu wrote:
>
>>> So, as a quick hack, if you make inline_frame_sniffer bail out early
>>> with 0, does it improve things?
>>
>
>> The timing looks the same. =(
>
> Bummer.  :-/
>
> Having now looked at the gdb.base/jit.exp test, I see that
> we can easily reproduce this with:
>
> gdb ---args ./testsuite/outputs/gdb.base/jit/jit-main ./testsuite/outputs/gdb.base/jit/jit-solib.so 2000
>
> (It'd be nice to turn this into a check-perf test...)

The new profile after disabling that line[1].

Am I reading it correctly that it skips the one I disabled and fails
to the next option which is basically as expensive?

Also, it looks like gdb is trying to get info about the current frame?
Can this be disabled for this breakpoint since it's not a "real" one?

[1] http://i.imgur.com/NHdoka8.jpg

>
>> What do you mean by "loader"? since you mentioned call JIT_REGISTER I
>> assume you mean JIT compiler?
>
> Yes, sorry.  I meant whatever code ends up "sending" a JIT_REGISTER
> event to gdb.
>
> Thanks,
> Pedro Alves
>

Re: JIT debugging (Attach and speed)

[Pedro Alves]

On 03/23/2016 08:51 PM, Yichao Yu wrote:

> Also, it looks like gdb is trying to get info about the current frame?
> Can this be disabled for this breakpoint since it's not a "real" one?

Excellent suggestion.  Seems like we can avoid computing a
frame unless we're stepping, or presenting a stop.

The prototype patch below (also in users/palves/jit-speedup, along
with the attach fixes), simply delays computing a frame until
it's necessary, and it cuts down the time for:

  gdb ---args ./testsuite/outputs/gdb.base/jit/jit-main ./testsuite/outputs/gdb.base/jit/jit-solib.so 2000

from "takes too long so I canceled it" minutes, to around 2 seconds, for me.

There's one hack though -- I commented out the skip_inline_frames
call...  We'll need to handle that some other way, or in some
other place.  As is, this regresses some inline tests, naturally...

Having a pending breakpoint that is never resolved slows things down
significantly again though...  The perf profile shows that breakpoint_re_set
ends up calling parse_linespec which wants the selected block, which wants
the selected frame, and then we're back at the same...  Maybe we
could reconsider how pending breakpoints are re-parsed, and which
context they're parsed in.  Not sure.

Still, very promising.

-----------
From a1e599495aafd10ae0505558343136f90b291038 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 23 Mar 2016 18:11:45 +0000
Subject: [PATCH] Avoid creating a frame in internal stops

This speeds up JIT library loading by ${a lot}.
---
 gdb/breakpoint.c |  3 +-
 gdb/infrun.c     | 98 ++++++++++++++++++++++++++++++--------------------------
 2 files changed, 54 insertions(+), 47 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f99a7ab..ed708a9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5792,8 +5792,7 @@ handle_jit_event (void)
      breakpoint_re_set.  */
   target_terminal_ours_for_output ();
 -  frame = get_current_frame ();
-  gdbarch = get_frame_arch (frame);
+  gdbarch = get_regcache_arch (get_current_regcache ());
    jit_event_handler (gdbarch);
 diff --git a/gdb/infrun.c b/gdb/infrun.c
index 696105d..6067cb0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5608,8 +5608,6 @@ finish_step_over (struct execution_control_state *ecs)
 static void
 handle_signal_stop (struct execution_control_state *ecs)
 {
-  struct frame_info *frame;
-  struct gdbarch *gdbarch;
   int stopped_by_watchpoint;
   enum stop_kind stop_soon;
   int random_signal;
@@ -5717,10 +5715,6 @@ handle_signal_stop (struct execution_control_state *ecs)
 	deprecated_context_hook (ptid_to_global_thread_id (ecs->ptid));
     }
 -  /* At this point, get hold of the now-current thread's frame.  */
-  frame = get_current_frame ();
-  gdbarch = get_frame_arch (frame);
-
   /* Pull the single step breakpoints out of the target.  */
   if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
     {
@@ -5774,7 +5768,7 @@ handle_signal_stop (struct execution_control_state *ecs)
      it in a moment.  */
   if (stopped_by_watchpoint
       && (target_have_steppable_watchpoint
-	  || gdbarch_have_nonsteppable_watchpoint (gdbarch)))
+	  || gdbarch_have_nonsteppable_watchpoint (get_regcache_arch (get_current_regcache ()))))
     {
       /* At this point, we are stopped at an instruction which has
          attempted to write to a piece of memory under control of
@@ -5838,27 +5832,25 @@ handle_signal_stop (struct execution_control_state *ecs)
 	 user had set a breakpoint on that inlined code, the missing
 	 skip_inline_frames call would break things.  Fortunately
 	 that's an extremely unlikely scenario.  */
+#if 0
       if (!pc_at_non_inline_function (aspace, stop_pc, &ecs->ws)
 	  && !(ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
 	       && ecs->event_thread->control.trap_expected
 	       && pc_at_non_inline_function (aspace,
 					     ecs->event_thread->prev_pc,
 					     &ecs->ws)))
-	{
-	  skip_inline_frames (ecs->ptid);
-
-	  /* Re-fetch current thread's frame in case that invalidated
-	     the frame cache.  */
-	  frame = get_current_frame ();
-	  gdbarch = get_frame_arch (frame);
-	}
+	skip_inline_frames (ecs->ptid);
+#endif
     }
    if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
       && ecs->event_thread->control.trap_expected
-      && gdbarch_single_step_through_delay_p (gdbarch)
+      && gdbarch_single_step_through_delay_p (get_regcache_arch (get_current_regcache ()))
       && currently_stepping (ecs->event_thread))
     {
+      struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+      struct frame_info *frame = get_current_frame ();
+
       /* We're trying to step off a breakpoint.  Turns out that we're
 	 also on an instruction that needs to be stepped multiple
 	 times before it's been fully executing.  E.g., architectures
@@ -5946,6 +5938,9 @@ handle_signal_stop (struct execution_control_state *ecs)
      been removed.  */
   if (random_signal && target_stopped_by_sw_breakpoint ())
     {
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+
       if (program_breakpoint_here_p (gdbarch, stop_pc))
 	{
 	  struct regcache *regcache;
@@ -6051,6 +6046,7 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  && ecs->event_thread->control.trap_expected
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
 	{
+	  struct frame_info *frame = get_current_frame ();
 	  int was_in_line;
  	  /* We were just starting a new sequence, attempting to
@@ -6105,11 +6101,14 @@ handle_signal_stop (struct execution_control_state *ecs)
        if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
 	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
-	      || ecs->event_thread->control.step_range_end == 1)
-	  && frame_id_eq (get_stack_frame_id (frame),
-			  ecs->event_thread->control.step_stack_frame_id)
-	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
+	      || ecs->event_thread->control.step_range_end == 1))
 	{
+	  struct frame_info *frame = get_current_frame ();
+
+	  if (frame_id_eq (get_stack_frame_id (frame),
+			   ecs->event_thread->control.step_stack_frame_id)
+	      && ecs->event_thread->control.step_resume_breakpoint == NULL)
+	    {
 	  /* The inferior is about to take a signal that will take it
 	     out of the single step range.  Set a breakpoint at the
 	     current PC (which is presumably where the signal handler
@@ -6131,6 +6130,7 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
 	  return;
+	    }
 	}
        /* Note: step_resume_breakpoint may be non-NULL.  This occures
@@ -6164,16 +6164,11 @@ static void
 process_event_stop_test (struct execution_control_state *ecs)
 {
   struct symtab_and_line stop_pc_sal;
-  struct frame_info *frame;
-  struct gdbarch *gdbarch;
   CORE_ADDR jmp_buf_pc;
   struct bpstat_what what;
    /* Handle cases caused by hitting a breakpoint.  */
 -  frame = get_current_frame ();
-  gdbarch = get_frame_arch (frame);
-
   what = bpstat_what (ecs->event_thread->control.stop_bpstat);
    if (what.call_dummy)
@@ -6185,12 +6180,6 @@ process_event_stop_test (struct execution_control_state *ecs)
      bp_jit_event).  Run them now.  */
   bpstat_run_callbacks (ecs->event_thread->control.stop_bpstat);
 -  /* If we hit an internal event that triggers symbol changes, the
-     current frame will be invalidated within bpstat_what (e.g., if we
-     hit an internal solib event).  Re-fetch it.  */
-  frame = get_current_frame ();
-  gdbarch = get_frame_arch (frame);
-
   switch (what.main_action)
     {
     case BPSTAT_WHAT_SET_LONGJMP_RESUME:
@@ -6207,6 +6196,8 @@ process_event_stop_test (struct execution_control_state *ecs)
       if (what.is_longjmp)
 	{
 	  struct value *arg_value;
+	  struct frame_info *frame = get_current_frame ();
+	  struct gdbarch *gdbarch = get_frame_arch (frame);
  	  /* If we set the longjmp breakpoint via a SystemTap probe,
 	     then use it to extract the arguments.  The destination PC
@@ -6233,7 +6224,11 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
 	}
       else
-	check_exception_resume (ecs, frame);
+	{
+	  struct frame_info *frame = get_current_frame ();
+
+	  check_exception_resume (ecs, frame);
+	}
       keep_going (ecs);
       return;
 @@ -6333,7 +6328,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  keep_going (ecs);
 	  return;
 	}
-      fill_in_stop_func (gdbarch, ecs);
+      fill_in_stop_func (get_regcache_arch (get_current_regcache ()), ecs);
       if (stop_pc == ecs->stop_func_start
 	  && execution_direction == EXEC_REVERSE)
 	{
@@ -6450,12 +6445,7 @@ process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 -  /* Re-fetch current thread's frame in case the code above caused
-     the frame cache to be re-initialized, making our FRAME variable
-     a dangling pointer.  */
-  frame = get_current_frame ();
-  gdbarch = get_frame_arch (frame);
-  fill_in_stop_func (gdbarch, ecs);
+  fill_in_stop_func (get_regcache_arch (get_current_regcache ()), ecs);
    /* If stepping through a line, keep going if still within it.
 @@ -6469,9 +6459,11 @@ process_event_stop_test (struct execution_control_state *ecs)
    if (pc_in_thread_step_range (stop_pc, ecs->event_thread)
       && (execution_direction != EXEC_REVERSE
-	  || frame_id_eq (get_frame_id (frame),
+	  || frame_id_eq (get_frame_id (get_current_frame ()),
 			  ecs->event_thread->control.step_frame_id)))
     {
+      struct gdbarch *gdbarch = get_frame_arch (get_current_frame ());
+
       if (debug_infrun)
 	fprintf_unfiltered
 	  (gdb_stdlog, "infrun: stepping inside range [%s-%s]\n",
@@ -6514,6 +6506,7 @@ process_event_stop_test (struct execution_control_state *ecs)
       && ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE
       && in_solib_dynsym_resolve_code (stop_pc))
     {
+      struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
       CORE_ADDR pc_after_resolver =
 	gdbarch_skip_solib_resolver (gdbarch, stop_pc);
 @@ -6529,7 +6522,7 @@ process_event_stop_test (struct execution_control_state *ecs)
  	  init_sal (&sr_sal);
 	  sr_sal.pc = pc_after_resolver;
-	  sr_sal.pspace = get_frame_program_space (frame);
+	  sr_sal.pspace = current_program_space;
  	  insert_step_resume_breakpoint_at_sal (gdbarch,
 						sr_sal, null_frame_id);
@@ -6542,7 +6535,7 @@ process_event_stop_test (struct execution_control_state *ecs)
   if (ecs->event_thread->control.step_range_end != 1
       && (ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE
 	  || ecs->event_thread->control.step_over_calls == STEP_OVER_ALL)
-      && get_frame_type (frame) == SIGTRAMP_FRAME)
+      && get_frame_type (get_current_frame ()) == SIGTRAMP_FRAME)
     {
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog,
@@ -6561,12 +6554,14 @@ process_event_stop_test (struct execution_control_state *ecs)
   /* macro/2012-04-25: This needs to come before the subroutine
      call check below as on some targets return trampolines look
      like subroutine calls (MIPS16 return thunks).  */
-  if (gdbarch_in_solib_return_trampoline (gdbarch,
+  if (gdbarch_in_solib_return_trampoline (get_regcache_arch (get_current_regcache ()),
 					  stop_pc, ecs->stop_func_name)
       && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
     {
       /* Determine where this trampoline returns.  */
       CORE_ADDR real_stop_pc;
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
        real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
 @@ -6615,7 +6610,7 @@ process_event_stop_test (struct execution_control_state *ecs)
      initial outermost frame, before sp was valid, would
      have code_addr == &_start.  See the comment in frame_id_eq
      for more.  */
-  if (!frame_id_eq (get_stack_frame_id (frame),
+  if (!frame_id_eq (get_stack_frame_id (get_current_frame ()),
 		    ecs->event_thread->control.step_stack_frame_id)
       && (frame_id_eq (frame_unwind_caller_id (get_current_frame ()),
 		       ecs->event_thread->control.step_stack_frame_id)
@@ -6625,6 +6620,8 @@ process_event_stop_test (struct execution_control_state *ecs)
 		  != find_pc_function (stop_pc)))))
     {
       CORE_ADDR real_stop_pc;
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
        if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog, "infrun: stepped into subroutine\n");
@@ -6787,6 +6784,9 @@ process_event_stop_test (struct execution_control_state *ecs)
   if (execution_direction == EXEC_REVERSE
       && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
     {
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+
       if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
 	  || (ecs->stop_func_start == 0
 	      && in_solib_dynsym_resolve_code (stop_pc)))
@@ -6825,6 +6825,9 @@ process_event_stop_test (struct execution_control_state *ecs)
       && ecs->stop_func_name == NULL
       && stop_pc_sal.line == 0)
     {
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog,
 			     "infrun: stepped into undebuggable function\n");
@@ -6969,7 +6972,12 @@ process_event_stop_test (struct execution_control_state *ecs)
   ecs->event_thread->control.step_range_start = stop_pc_sal.pc;
   ecs->event_thread->control.step_range_end = stop_pc_sal.end;
   ecs->event_thread->control.may_range_step = 1;
-  set_step_info (frame, stop_pc_sal);
+
+  {
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+      set_step_info (frame, stop_pc_sal);
+  }
    if (debug_infrun)
      fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n");
-- 
2.5.5

Re: JIT debugging (Attach and speed)

[Yichao Yu]

> The prototype patch below (also in users/palves/jit-speedup, along
> with the attach fixes), simply delays computing a frame until
> it's necessary, and it cuts down the time for:

(The patch seems to have some white space breakage but the branch works great.)

>   gdb ---args ./testsuite/outputs/gdb.base/jit/jit-main ./testsuite/outputs/gdb.base/jit/jit-solib.so 2000
>
> from "takes too long so I canceled it" minutes, to around 2 seconds, for me.

There's indeed a huge improvement. The test I was using with ~2000 JIT
objects goes drops from ~400s to 7.5s (~3s without gdb). However a
rough scaling test suggests that the scaling is still ~O(n^2) 20k JIT
objects takes ~500s (~130s without gdb). It seems that the master was
actually super quadratic (if this is a word) when dealing with
multiple JIT objects.

> Having a pending breakpoint that is never resolved slows things down
> significantly again though...  The perf profile shows that breakpoint_re_set
> ends up calling parse_linespec which wants the selected block, which wants
> the selected frame, and then we're back at the same...  Maybe we
> could reconsider how pending breakpoints are re-parsed, and which
> context they're parsed in.  Not sure.
>
> Still, very promising.
>

Indeed. It's already so much better.

Re: JIT debugging (Attach and speed)

[Yichao Yu]

> There's indeed a huge improvement. The test I was using with ~2000 JIT
> objects goes drops from ~400s to 7.5s (~3s without gdb). However a
> rough scaling test suggests that the scaling is still ~O(n^2) 20k JIT
> objects takes ~500s (~130s without gdb). It seems that the master was
> actually super quadratic (if this is a word) when dealing with
> multiple JIT objects.

The new profile is [1] and it is indeed much closer to the one in 2011.

[1] http://i.imgur.com/K3GKYeq.jpg

>
>> Having a pending breakpoint that is never resolved slows things down
>> significantly again though...  The perf profile shows that breakpoint_re_set
>> ends up calling parse_linespec which wants the selected block, which wants
>> the selected frame, and then we're back at the same...  Maybe we
>> could reconsider how pending breakpoints are re-parsed, and which
>> context they're parsed in.  Not sure.
>>
>> Still, very promising.
>>
>
> Indeed. It's already so much better.