[v2,3/4] malloc: Use hp-timing on libmemusage
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
Instead of reimplemeting on GETTIME macro.
---
malloc/memusage.c | 15 ++++++++++++---
sysdeps/generic/memusage.h | 12 ------------
sysdeps/i386/i686/memusage.h | 1 -
sysdeps/ia64/memusage.h | 7 -------
sysdeps/x86_64/memusage.h | 1 -
5 files changed, 12 insertions(+), 24 deletions(-)
Comments
On Thu, Dec 23, 2021 at 02:37:05PM -0300, Adhemerval Zanella via Libc-alpha wrote:
> Instead of reimplemeting on GETTIME macro.
> ---
> malloc/memusage.c | 15 ++++++++++++---
> sysdeps/generic/memusage.h | 12 ------------
> sysdeps/i386/i686/memusage.h | 1 -
> sysdeps/ia64/memusage.h | 7 -------
> sysdeps/x86_64/memusage.h | 1 -
> 5 files changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/malloc/memusage.c b/malloc/memusage.c
> index de39ad1c1a..179dd1c683 100644
> --- a/malloc/memusage.c
> +++ b/malloc/memusage.c
> @@ -34,6 +34,7 @@
> #include <sys/time.h>
>
> #include <memusage.h>
> +#include <hp-timing.h>
>
> /* Pointer to the real functions. These are determined used `dlsym'
> when really needed. */
> @@ -114,6 +115,14 @@ static struct entry buffer[2 * DEFAULT_BUFFER_SIZE];
> static uint32_t buffer_cnt;
> static struct entry first;
>
> +static void
> +gettime (struct entry *e)
> +{
> + hp_timing_t now;
> + HP_TIMING_NOW (now);
> + e->time_low = now & 0xffffffff;
> + e->time_high = now >> 32;
> +}
OK. this matches the generic definitions.
> /* Update the global data after a successful function call. */
> static void
> @@ -177,7 +186,7 @@ update_data (struct header *result, size_t len, size_t old_len)
>
> buffer[idx].heap = current_heap;
> buffer[idx].stack = current_stack;
> - GETTIME (buffer[idx].time_low, buffer[idx].time_high);
> + gettime (&buffer[idx]);
>
> /* Write out buffer if it is full. */
> if (idx + 1 == buffer_size)
> @@ -267,7 +276,7 @@ me (void)
> /* Write the first entry. */
> first.heap = 0;
> first.stack = 0;
> - GETTIME (first.time_low, first.time_high);
> + gettime (&first);
> /* Write it two times since we need the starting and end time. */
> write (fd, &first, sizeof (first));
> write (fd, &first, sizeof (first));
> @@ -818,7 +827,7 @@ dest (void)
> stack. */
> first.heap = peak_heap;
> first.stack = peak_stack;
> - GETTIME (first.time_low, first.time_high);
> + gettime (&first);
> write (fd, &first, sizeof (struct entry));
>
> /* Close the file. */
OK.
> diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
> index c9bde5cd11..514bd058d2 100644
> --- a/sysdeps/generic/memusage.h
> +++ b/sysdeps/generic/memusage.h
> @@ -23,15 +23,3 @@
> # warning "GETSP is not defined for this architecture."
> # define GETSP 0
> #endif
> -
> -#ifndef GETTIME
> -# define GETTIME(low,high) \
> - { \
> - struct __timespec64 now; \
> - uint64_t usecs; \
> - __clock_gettime64 (CLOCK_REALTIME, &now); \
> - usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
> - low = usecs & 0xffffffff; \
> - high = usecs >> 32; \
> - }
> -#endif
Replacing with genric definition of GET_TIME_NOW...
#ifdef _ISOMAC
# define HP_TIMING_NOW(var) \
({ \
struct timespec tv; \
clock_gettime (CLOCK_MONOTONIC, &tv); \
(var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec); \
})
#else
# define HP_TIMING_NOW(var) \
({ \
struct __timespec64 tv; \
__clock_gettime64 (CLOCK_MONOTONIC, &tv); \
(var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec); \
})
#endif
This is now CLOCK_MONOTONIC rather than CLOCK_REALTIME. Also it returns
nanoseconds rather than microseconds.
I think this is OK though as it seems the time unit in memusage is not
defined.
> diff --git a/sysdeps/i386/i686/memusage.h b/sysdeps/i386/i686/memusage.h
> index 08b7831e12..07b241263c 100644
> --- a/sysdeps/i386/i686/memusage.h
> +++ b/sysdeps/i386/i686/memusage.h
> @@ -16,6 +16,5 @@
> <https://www.gnu.org/licenses/>. */
>
> #define GETSP() ({ register uintptr_t stack_ptr asm ("esp"); stack_ptr; })
> -#define GETTIME(low,high) asm ("rdtsc" : "=a" (low), "=d" (high))
>
> #include <sysdeps/generic/memusage.h>
> diff --git a/sysdeps/ia64/memusage.h b/sysdeps/ia64/memusage.h
> index 3c3a526007..33fd6ec899 100644
> --- a/sysdeps/ia64/memusage.h
> +++ b/sysdeps/ia64/memusage.h
> @@ -18,12 +18,5 @@
> #include <hp-timing.h>
>
> #define GETSP() ({ register uintptr_t stack_ptr asm ("%r12"); stack_ptr; })
> -#define GETTIME(low, high) \
> - { \
> - hp_timing_t __now; \
> - HP_TIMING_NOW (__now); \
> - low = __now & 0xffffffff; \
> - high = __now >> 32; \
> - }
>
> #include <sysdeps/generic/memusage.h>
> diff --git a/sysdeps/x86_64/memusage.h b/sysdeps/x86_64/memusage.h
> index 568dd5512d..6652fc5da1 100644
> --- a/sysdeps/x86_64/memusage.h
> +++ b/sysdeps/x86_64/memusage.h
> @@ -16,6 +16,5 @@
> <https://www.gnu.org/licenses/>. */
>
> #define GETSP() ({ register uintptr_t stack_ptr asm ("rsp"); stack_ptr; })
> -#define GETTIME(low,high) asm ("rdtsc" : "=a" (low), "=d" (high))
>
> #include <sysdeps/generic/memusage.h>
-Stafford
On 27/12/2021 09:40, Stafford Horne wrote:
>> diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
>> index c9bde5cd11..514bd058d2 100644
>> --- a/sysdeps/generic/memusage.h
>> +++ b/sysdeps/generic/memusage.h
>> @@ -23,15 +23,3 @@
>> # warning "GETSP is not defined for this architecture."
>> # define GETSP 0
>> #endif
>> -
>> -#ifndef GETTIME
>> -# define GETTIME(low,high) \
>> - { \
>> - struct __timespec64 now; \
>> - uint64_t usecs; \
>> - __clock_gettime64 (CLOCK_REALTIME, &now); \
>> - usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
>> - low = usecs & 0xffffffff; \
>> - high = usecs >> 32; \
>> - }
>> -#endif
>
> Replacing with genric definition of GET_TIME_NOW...
>
> #ifdef _ISOMAC
> # define HP_TIMING_NOW(var) \
> ({ \
> struct timespec tv; \
> clock_gettime (CLOCK_MONOTONIC, &tv); \
> (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec); \
> })
> #else
> # define HP_TIMING_NOW(var) \
> ({ \
> struct __timespec64 tv; \
> __clock_gettime64 (CLOCK_MONOTONIC, &tv); \
> (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec); \
> })
> #endif
>
> This is now CLOCK_MONOTONIC rather than CLOCK_REALTIME. Also it returns
> nanoseconds rather than microseconds.
>
> I think this is OK though as it seems the time unit in memusage is not
> defined.
Right, I did not realize the clock id change. I will change to keep current
behavior:
static void
gettime (struct entry *e)
{
#if HP_TIMING_INLINE
hp_timing_t now;
HP_TIMING_NOW (now);
e->time_low = now & 0xffffffff;
e->time_high = now >> 32;
#else
struct __timespec64 now;
uint64_t usecs;
__clock_gettime64 (CLOCK_REALTIME, &now);
usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000;
e->time_low = usecs & 0xffffffff;
e->time_high = usecs >> 32;
#endif
}
@@ -34,6 +34,7 @@
#include <sys/time.h>
#include <memusage.h>
+#include <hp-timing.h>
/* Pointer to the real functions. These are determined used `dlsym'
when really needed. */
@@ -114,6 +115,14 @@ static struct entry buffer[2 * DEFAULT_BUFFER_SIZE];
static uint32_t buffer_cnt;
static struct entry first;
+static void
+gettime (struct entry *e)
+{
+ hp_timing_t now;
+ HP_TIMING_NOW (now);
+ e->time_low = now & 0xffffffff;
+ e->time_high = now >> 32;
+}
/* Update the global data after a successful function call. */
static void
@@ -177,7 +186,7 @@ update_data (struct header *result, size_t len, size_t old_len)
buffer[idx].heap = current_heap;
buffer[idx].stack = current_stack;
- GETTIME (buffer[idx].time_low, buffer[idx].time_high);
+ gettime (&buffer[idx]);
/* Write out buffer if it is full. */
if (idx + 1 == buffer_size)
@@ -267,7 +276,7 @@ me (void)
/* Write the first entry. */
first.heap = 0;
first.stack = 0;
- GETTIME (first.time_low, first.time_high);
+ gettime (&first);
/* Write it two times since we need the starting and end time. */
write (fd, &first, sizeof (first));
write (fd, &first, sizeof (first));
@@ -818,7 +827,7 @@ dest (void)
stack. */
first.heap = peak_heap;
first.stack = peak_stack;
- GETTIME (first.time_low, first.time_high);
+ gettime (&first);
write (fd, &first, sizeof (struct entry));
/* Close the file. */
@@ -23,15 +23,3 @@
# warning "GETSP is not defined for this architecture."
# define GETSP 0
#endif
-
-#ifndef GETTIME
-# define GETTIME(low,high) \
- { \
- struct __timespec64 now; \
- uint64_t usecs; \
- __clock_gettime64 (CLOCK_REALTIME, &now); \
- usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
- low = usecs & 0xffffffff; \
- high = usecs >> 32; \
- }
-#endif
@@ -16,6 +16,5 @@
<https://www.gnu.org/licenses/>. */
#define GETSP() ({ register uintptr_t stack_ptr asm ("esp"); stack_ptr; })
-#define GETTIME(low,high) asm ("rdtsc" : "=a" (low), "=d" (high))
#include <sysdeps/generic/memusage.h>
@@ -18,12 +18,5 @@
#include <hp-timing.h>
#define GETSP() ({ register uintptr_t stack_ptr asm ("%r12"); stack_ptr; })
-#define GETTIME(low, high) \
- { \
- hp_timing_t __now; \
- HP_TIMING_NOW (__now); \
- low = __now & 0xffffffff; \
- high = __now >> 32; \
- }
#include <sysdeps/generic/memusage.h>
@@ -16,6 +16,5 @@
<https://www.gnu.org/licenses/>. */
#define GETSP() ({ register uintptr_t stack_ptr asm ("rsp"); stack_ptr; })
-#define GETTIME(low,high) asm ("rdtsc" : "=a" (low), "=d" (high))
#include <sysdeps/generic/memusage.h>