* [RFC 0/2 gdbserver] Compute 'traceframe usage' per tracepoint
@ 2012-12-12 1:37 Yao Qi
2012-12-12 1:38 ` [PATCH 1/2] Test case: check traceframe_usage Yao Qi
2012-12-12 1:38 ` [PATCH 2/2] Compute traceframe usuage per tracepoint on demand Yao Qi
0 siblings, 2 replies; 6+ messages in thread
From: Yao Qi @ 2012-12-12 1:37 UTC (permalink / raw)
To: gdb-patches
Hi,
I happen to see that field 'traceframe_usage' in tracepoint in GDB is
always zero, because its value is from GDBserver and GDBserver doesn't
get a correct one of 'traceframe_usage'. The 'traceframe usage' is
always zero when 'traceframe_usage' was added in this patch,
[PATCH v2] Tracing notes and metadata
http://sourceware.org/ml/gdb-patches/2011-11/msg00484.html
The patch 2/2 fixes this problem by computing the traceframe usage when
replying to packet 'qTP'.
The other approach of fixing this problem
is to continue to use field 'traceframe_usage' of 'struct tracepoint'
and accumulate it when adding a block to a traceframe
(in add_traceframe_block). Each 'traceframe' has the number of
tracepoint, but it is not a good way to iterate the tracepoint list
to find the right tracepoint and increment its field
'traceframe_usuage' inside add_traceframe_block, which should be done
as fast it could be. Then, I thought that we may use pointer to
'struct tracepoint' instead of tracepoint num in 'struct traceframe',
but comments of 'struct traceframe' tell me that I shouldn't do this,
"This object should be as small as possible". If we don't mind
increasing 2 bytes (on 32-bit target) for each 'struct traceframe',
I am OK to replace 'tpnum' with a pointer to 'struct tracepoint',
and post the patches in the other approach.
If we follow the approach of this patch, that means we don't need
the field 'traceframe_usage' in 'struct tracepoint' in GDBserver,
and we can remove it. What do you think?
Test case: check traceframe_usage.
Compute traceframe usuage per tracepoint on demand.
gdb/gdbserver/tracepoint.c | 12 ++++++++-
gdb/testsuite/gdb.trace/disconnected-tracing.c | 8 ++++++
gdb/testsuite/gdb.trace/disconnected-tracing.exp | 29 ++++++++++++++++++----
gdb/testsuite/gdb.trace/infotrace.exp | 27 ++++++++++++++++++++
4 files changed, 70 insertions(+), 6 deletions(-)
--
1.7.7.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] Test case: check traceframe_usage.
2012-12-12 1:37 [RFC 0/2 gdbserver] Compute 'traceframe usage' per tracepoint Yao Qi
@ 2012-12-12 1:38 ` Yao Qi
2012-12-12 1:38 ` [PATCH 2/2] Compute traceframe usuage per tracepoint on demand Yao Qi
1 sibling, 0 replies; 6+ messages in thread
From: Yao Qi @ 2012-12-12 1:38 UTC (permalink / raw)
To: gdb-patches
Hi,
This patch adds a test to check the output of 'info tracepoint' for
traceframe_usage per tracepoint. Note that user has to type 'tstatus'
command first to get the latest tracepoint status in GDB side.
gdb/testsuite:
2012-12-11 Yao Qi <yao@codesourcery.com>
* gdb.trace/infotrace.exp: Check 'traceframe usage' in the
output of 'info tracepoints'.
* gdb.trace/disconnected-tracing.exp (disconnected_tracing):
Likewise.
* gdb.trace/disconnected-tracing.c (struct foo): New.
---
gdb/testsuite/gdb.trace/disconnected-tracing.c | 8 ++++++
gdb/testsuite/gdb.trace/disconnected-tracing.exp | 29 ++++++++++++++++++----
gdb/testsuite/gdb.trace/infotrace.exp | 27 ++++++++++++++++++++
3 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/gdb/testsuite/gdb.trace/disconnected-tracing.c b/gdb/testsuite/gdb.trace/disconnected-tracing.c
index d4f54b2..13badec 100644
--- a/gdb/testsuite/gdb.trace/disconnected-tracing.c
+++ b/gdb/testsuite/gdb.trace/disconnected-tracing.c
@@ -19,6 +19,14 @@ void
end (void)
{}
+struct foo
+{
+ int bar1;
+ long bar2;
+};
+
+struct foo foo;
+
void
start (void)
{}
diff --git a/gdb/testsuite/gdb.trace/disconnected-tracing.exp b/gdb/testsuite/gdb.trace/disconnected-tracing.exp
index ed6bfd4..6640b50 100644
--- a/gdb/testsuite/gdb.trace/disconnected-tracing.exp
+++ b/gdb/testsuite/gdb.trace/disconnected-tracing.exp
@@ -47,6 +47,8 @@ if ![gdb_target_supports_trace] {
proc disconnected_tracing { } { with_test_prefix "trace" {
global executable
+ global decimal
+ global srcfile
# Start with a fresh gdb.
clean_restart ${executable}
@@ -57,10 +59,18 @@ proc disconnected_tracing { } { with_test_prefix "trace" {
gdb_test_no_output "set confirm off"
gdb_test_no_output "set disconnected-tracing on"
- gdb_test "trace main" ".*"
+ gdb_test "trace start" ".*"
+ gdb_trace_setactions "collect on tracepoint 2" "2" \
+ "collect foo" "^$"
+ gdb_test "break end" "Breakpoint ${decimal} at .*"
+
gdb_test_no_output "tstart"
- gdb_test "info tracepoints" ".*in main at.*" "first info tracepoints"
+ gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*"
+ gdb_test_no_output "tstop"
+
+ gdb_test "info tracepoints" ".*in start at.*" \
+ "first info tracepoints"
gdb_test "disconnect" "Ending remote debugging\\." "first disconnect"
if { [gdb_reconnect] == 0 } {
@@ -69,10 +79,12 @@ proc disconnected_tracing { } { with_test_prefix "trace" {
fail "first reconnect after unload"
return 0
}
- gdb_test "info tracepoints" ".*in main at.*" "second info tracepoints"
+ gdb_test "info tracepoints" ".*in start at.*" \
+ "second info tracepoints"
delete_breakpoints
- gdb_test "info tracepoints" ".*No tracepoints..*" "third info tracepoints"
+ gdb_test "info tracepoints" ".*No tracepoints..*" \
+ "third info tracepoints"
gdb_test "disconnect" "Ending remote debugging\\." "second disconnect"
if { [gdb_reconnect] == 0 } {
@@ -81,7 +93,14 @@ proc disconnected_tracing { } { with_test_prefix "trace" {
fail "second reconnect after unload"
return 0
}
- gdb_test "info tracepoints" ".*in main at.*" "fourth info tracepoints"
+ gdb_test "tstatus"
+ gdb_test "info tracepoints" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+tracepoint keep y.* in start at .*$srcfile:\[0-9\]+.
+\[\t \]+tracepoint already hit 1 time.
+\[\t \]+trace buffer usage ${decimal} bytes.
+\[\t \]+collect foo.*" \
+ "fourth info tracepoints"
}}
disconnected_tracing
diff --git a/gdb/testsuite/gdb.trace/infotrace.exp b/gdb/testsuite/gdb.trace/infotrace.exp
index bdc3830..5cfa1e8 100644
--- a/gdb/testsuite/gdb.trace/infotrace.exp
+++ b/gdb/testsuite/gdb.trace/infotrace.exp
@@ -85,3 +85,30 @@ gdb_test "help info tracepoints" \
"Status of specified tracepoints .all tracepoints if no argument.*" \
"2.5: help info tracepoints"
+# 2.6 info tracepoints (check trace buffer usage). We need a live
+# tracing.
+gdb_breakpoint "main"
+gdb_trace_setactions "collect on tracepoint 1" "1" \
+ "collect gdb_struct1_test" "^$"
+gdb_run_cmd
+gdb_test "" "Breakpoint ${decimal}, main.*"
+
+if { ![gdb_target_supports_trace] } then {
+ unsupported "Current target does not support trace"
+ return 1;
+}
+
+gdb_test "break end" "Breakpoint \[0-9\] at .*"
+gdb_test_no_output "tstart"
+gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" \
+ "continue to end"
+gdb_test_no_output "tstop"
+gdb_test "tstatus"
+gdb_test "info tracepoints" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+tracepoint keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+tracepoint already hit 1 time.
+\[\t \]+trace buffer usage ${decimal} bytes.
+\[\t \]+collect gdb_struct1_test.
+\[0-9\]+\[\t \]+tracepoint keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+." \
+ "2.6: info tracepoints (trace buffer usage)"
--
1.7.7.6
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] Compute traceframe usuage per tracepoint on demand.
2012-12-12 1:37 [RFC 0/2 gdbserver] Compute 'traceframe usage' per tracepoint Yao Qi
2012-12-12 1:38 ` [PATCH 1/2] Test case: check traceframe_usage Yao Qi
@ 2012-12-12 1:38 ` Yao Qi
2012-12-17 9:21 ` Yao Qi
1 sibling, 1 reply; 6+ messages in thread
From: Yao Qi @ 2012-12-12 1:38 UTC (permalink / raw)
To: gdb-patches
Hi,
This patch fixes this problem by computing the traceframe usage when
replying to packet 'qTP'.
gdb/gdbserver:
2012-12-11 Yao Qi <yao@codesourcery.com>
* tracepoint.c (cmd_qtp): Compute the traceframe usage of a
tracepoint.
---
gdb/gdbserver/tracepoint.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 1526838..ecc85ab 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3674,6 +3674,8 @@ cmd_qtp (char *own_buf)
ULONGEST num, addr;
struct tracepoint *tpoint;
char *packet = own_buf;
+ struct traceframe *tframe;
+ uint64_t traceframe_usage = 0;
packet += strlen ("qTP:");
@@ -3692,8 +3694,16 @@ cmd_qtp (char *own_buf)
return;
}
+ /* Compute the traceframe usage of tracepoint whose number is
+ NUM. */
+ for (tframe = FIRST_TRACEFRAME ();
+ tframe->tpnum != 0;
+ tframe = NEXT_TRACEFRAME (tframe))
+ if (tframe->tpnum == num)
+ traceframe_usage += tframe->data_size;
+
sprintf (own_buf, "V%" PRIu64 ":%" PRIu64 "", tpoint->hit_count,
- tpoint->traceframe_usage);
+ traceframe_usage);
}
/* State variables to help return all the tracepoint bits. */
--
1.7.7.6
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] Compute traceframe usuage per tracepoint on demand.
2012-12-12 1:38 ` [PATCH 2/2] Compute traceframe usuage per tracepoint on demand Yao Qi
@ 2012-12-17 9:21 ` Yao Qi
2013-01-07 15:39 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2012-12-17 9:21 UTC (permalink / raw)
To: gdb-patches
On 12/12/2012 09:37 AM, Yao Qi wrote:
> + /* Compute the traceframe usage of tracepoint whose number is
> + NUM. */
> + for (tframe = FIRST_TRACEFRAME ();
> + tframe->tpnum != 0;
> + tframe = NEXT_TRACEFRAME (tframe))
> + if (tframe->tpnum == num)
> + traceframe_usage += tframe->data_size;
> +
> sprintf (own_buf, "V%" PRIu64 ":%" PRIu64 "", tpoint->hit_count,
> - tpoint->traceframe_usage);
> + traceframe_usage);
It is incorrect for tracepoint with multiple locations, because multiple
tracepoints in GDBserver have the same tracepoint number, and each
tracepoint has its own traceframe usage.
On 12/12/2012 09:37 AM, Yao Qi wrote:
> Then, I thought that we may use pointer to
> 'struct tracepoint' instead of tracepoint num in 'struct traceframe',
> but comments of 'struct traceframe' tell me that I shouldn't do this,
> "This object should be as small as possible". If we don't mind
> increasing 2 bytes (on 32-bit target) for each 'struct traceframe',
Looks we have to go this way, in order to get the correct traceframe
usage of each tracepoint in GDBserver.
I'll post a new patch again.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Compute traceframe usuage per tracepoint on demand.
2012-12-17 9:21 ` Yao Qi
@ 2013-01-07 15:39 ` Pedro Alves
2013-01-07 15:56 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2013-01-07 15:39 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 12/17/2012 09:20 AM, Yao Qi wrote:
> On 12/12/2012 09:37 AM, Yao Qi wrote:
>> + /* Compute the traceframe usage of tracepoint whose number is
>> + NUM. */
>> + for (tframe = FIRST_TRACEFRAME ();
>> + tframe->tpnum != 0;
>> + tframe = NEXT_TRACEFRAME (tframe))
>> + if (tframe->tpnum == num)
>> + traceframe_usage += tframe->data_size;
>> +
>> sprintf (own_buf, "V%" PRIu64 ":%" PRIu64 "", tpoint->hit_count,
>> - tpoint->traceframe_usage);
>> + traceframe_usage);
>
> It is incorrect for tracepoint with multiple locations, because multiple tracepoints in GDBserver have the same tracepoint number, and each tracepoint has its own traceframe usage.
>
> On 12/12/2012 09:37 AM, Yao Qi wrote:
>> Then, I thought that we may use pointer to
>> 'struct tracepoint' instead of tracepoint num in 'struct traceframe',
>> but comments of 'struct traceframe' tell me that I shouldn't do this,
>> "This object should be as small as possible". If we don't mind
>> increasing 2 bytes (on 32-bit target) for each 'struct traceframe',
>
> Looks we have to go this way, in order to get the correct traceframe usage of each tracepoint in GDBserver.
To avoid the size penalty on 64-bit, another option would be
for gdbserver to maintain an internal tracepoint number, unique
for each location, and for traceframes to record that instead
of gdb tracepoint number.
Did you try the other option of accounting for usage in
add_traceframe_block? I suspect it may be simple and not
add much more than a few instructions.
> I'll post a new patch again.
I may have missed it. Have you posted it?
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Compute traceframe usuage per tracepoint on demand.
2013-01-07 15:39 ` Pedro Alves
@ 2013-01-07 15:56 ` Pedro Alves
0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2013-01-07 15:56 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 01/07/2013 03:39 PM, Pedro Alves wrote:
> On 12/17/2012 09:20 AM, Yao Qi wrote:
>> It is incorrect for tracepoint with multiple locations, because multiple
>> tracepoints in GDBserver have the same tracepoint number, and
>> each tracepoint has its own traceframe usage.
>> On 12/12/2012 09:37 AM, Yao Qi wrote:
>>> Then, I thought that we may use pointer to
>>> 'struct tracepoint' instead of tracepoint num in 'struct traceframe',
>>> but comments of 'struct traceframe' tell me that I shouldn't do this,
>>> "This object should be as small as possible". If we don't mind
>>> increasing 2 bytes (on 32-bit target) for each 'struct traceframe',
>>
>> Looks we have to go this way, in order to get the correct traceframe usage of each tracepoint in GDBserver.
>
> To avoid the size penalty on 64-bit, another option would be
> for gdbserver to maintain an internal tracepoint number, unique
> for each location, and for traceframes to record that instead
> of gdb tracepoint number.
Actually, I just remembered that "tfind tracepoint" is
broken for multiple locations. Notice:
(gdb) tfind tracepoint 1.1
Sending packet: $QTFrame:tdp:1#7d...Packet received: F-1
...
(gdb) tfind tracepoint 1.2
Sending packet: $QTFrame:tdp:1#7d...Packet received: F-1
...
"QTFrame:tdp:" only sends the tracepoint number, not the
address, so the remote side can't distinguish locations...
Doesn't strictly speak against doing the accounting in
add_traceframe_block, but, in order to fix that we will need
to change struct traceframe anyhow.
>
> Did you try the other option of accounting for usage in
> add_traceframe_block? I suspect it may be simple and not
> add much more than a few instructions.
>
>> I'll post a new patch again.
>
> I may have missed it. Have you posted it?
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-07 15:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-12 1:37 [RFC 0/2 gdbserver] Compute 'traceframe usage' per tracepoint Yao Qi
2012-12-12 1:38 ` [PATCH 1/2] Test case: check traceframe_usage Yao Qi
2012-12-12 1:38 ` [PATCH 2/2] Compute traceframe usuage per tracepoint on demand Yao Qi
2012-12-17 9:21 ` Yao Qi
2013-01-07 15:39 ` Pedro Alves
2013-01-07 15:56 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox