From b0e640a505171550746af20e940076c579a0e638 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Mon, 4 Nov 2013 14:48:01 -0800 Subject: [PATCH] Remove the context_get_current function This completes the recent re-factoring begun a few commits ago. Now the layer violations are eliminated, with the metrics.c code always accepting a metrics_t* and never reachin up into context.c to find a global data structure. --- context.c | 35 +++++++++++-- context.h | 44 +++++++++++++++-- eglwrap.c | 6 +-- glwrap.c | 10 ++-- glxwrap.c | 6 +-- metrics.c | 144 +++++++++++++----------------------------------------- metrics.h | 12 ++--- 7 files changed, 121 insertions(+), 136 deletions(-) diff --git a/context.c b/context.c index ccaa96a..933e1ed 100644 --- a/context.c +++ b/context.c @@ -61,8 +61,9 @@ context_enter (fips_api_t api, void *system_context_id) current_context = context_create (api, system_context_id); - metrics_set_current_op (METRICS_OP_SHADER + 0); - metrics_counter_start (); + metrics_set_current_op (current_context->metrics, + METRICS_OP_SHADER + 0); + metrics_counter_start (current_context->metrics); } void @@ -76,8 +77,32 @@ context_leave (void) metrics_destroy (ctx->metrics); } -context_t * -context_get_current (void) +void +context_counter_start (void) +{ + metrics_counter_start (current_context->metrics); +} + +void +context_counter_stop (void) +{ + metrics_counter_stop (current_context->metrics); +} + +void +context_set_current_op (metrics_op_t op) +{ + metrics_set_current_op (current_context->metrics, op); +} + +metrics_op_t +context_get_current_op (void) +{ + return metrics_get_current_op (current_context->metrics); +} + +void +context_end_frame (void) { - return current_context; + return metrics_end_frame (current_context->metrics); } diff --git a/context.h b/context.h index 1fddf2e..071b5f4 100644 --- a/context.h +++ b/context.h @@ -53,8 +53,46 @@ context_enter (fips_api_t api, void *system_context_id); void context_leave (void); -/* Get the current context. */ -context_t * -context_get_current (void); +/* Start accumulating GPU time. + * + * The time accumulated will be accounted against the + * current operation (as set with context_set_current_op). + */ +void +context_counter_start (void); + +/* Stop accumulating GPU time (stops the most-recently started counter) */ +void +context_counter_stop (void); + +/* Set a metrics_op_t value to indicate what kind of operation is + * being performed. + * + * The metrics-tracking code will account for timings by accumulating + * measured counter values into a separate counter for each + * metrics_op_t value, (so that the report can describe which + * operations are the most expensive). + * + * In addition, for the value METRICS_OP_SHADER, each specific shader + * program can be distinguished. To accomplish this, pass a value of + * METRICS_OP_SHADER + shader_program_number to this function. + */ +void +context_set_current_op (metrics_op_t op); + +/* Return the current metrics_op_t value, (the value most-recently-set + * with a call to context_set_current_op). + */ +metrics_op_t +context_get_current_op (void); + +/* Should be called at the end of every function wrapper for a + * function that ends a frame, (glXSwapBuffers and similar). + * + * This function performs whatever bookkeeping is necessary to + * generate a timing report, then emits that report. + */ +void +context_end_frame (void); #endif diff --git a/eglwrap.c b/eglwrap.c index fe7f736..913532e 100644 --- a/eglwrap.c +++ b/eglwrap.c @@ -80,11 +80,11 @@ eglSwapBuffers (EGLDisplay dpy, EGLSurface surface) EGLWRAP_DEFER_WITH_RETURN (ret, eglSwapBuffers, dpy, surface); - metrics_counter_stop (); + context_counter_stop (); - metrics_end_frame (); + context_end_frame (); - metrics_counter_start (); + context_counter_start (); return ret; } diff --git a/glwrap.c b/glwrap.c index 22d4808..39a784f 100644 --- a/glwrap.c +++ b/glwrap.c @@ -45,7 +45,7 @@ #include "glwrap.h" -#include "metrics.h" +#include "context.h" /* As of glext.h version 20131008 some types changed. * @@ -66,13 +66,13 @@ static void *gl_handle; /* Switch metrics operation persistently, (until next SWITCH) */ #define SWITCH_METRICS_OP(op) \ - metrics_counter_stop (); \ - metrics_set_current_op (op); \ - metrics_counter_start (); + context_counter_stop (); \ + context_set_current_op (op); \ + context_counter_start (); /* Switch metrics operation temporarily, see RESTORE_METRICS_OP */ #define SAVE_THEN_SWITCH_METRICS_OP(op) \ - metrics_op_t save = metrics_get_current_op (); \ + metrics_op_t save = context_get_current_op (); \ SWITCH_METRICS_OP (op); /* Switch back to metrics operation saved by SAVE_THEN_SWITCH_METRICS_OP */ diff --git a/glxwrap.c b/glxwrap.c index 4545d6f..4b87642 100644 --- a/glxwrap.c +++ b/glxwrap.c @@ -37,11 +37,11 @@ glXSwapBuffers (Display *dpy, GLXDrawable drawable) { GLWRAP_DEFER (glXSwapBuffers, dpy, drawable); - metrics_counter_stop (); + context_counter_stop (); - metrics_end_frame (); + context_end_frame (); - metrics_counter_start (); + context_counter_start (); } /* glXGetProcAddressARB is a function which accepts a string and diff --git a/metrics.c b/metrics.c index 47df1e0..6704572 100644 --- a/metrics.c +++ b/metrics.c @@ -59,7 +59,7 @@ typedef struct monitor typedef struct op_metrics { /* This happens to also be the index into the - * ctx->op_metrics array currently + * metrics->op_metrics array currently */ metrics_op_t op; double time_ns; @@ -227,10 +227,8 @@ metrics_op_string (metrics_op_t op) } void -metrics_counter_start (void) +metrics_counter_start (metrics_t *metrics) { - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; unsigned i; /* Initialize the timer_query and monitor objects */ @@ -268,10 +266,8 @@ metrics_counter_start (void) } void -metrics_counter_stop (void) +metrics_counter_stop (metrics_t *metrics) { - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; timer_query_t *timer; monitor_t *monitor; @@ -318,31 +314,24 @@ metrics_counter_stop (void) * available by now.) */ if (metrics->monitors_in_flight > MAX_MONITORS_IN_FLIGHT) - metrics_collect_available (); + metrics_collect_available (metrics); } void -metrics_set_current_op (metrics_op_t op) +metrics_set_current_op (metrics_t *metrics, metrics_op_t op) { - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; - metrics->op = op; } metrics_op_t -metrics_get_current_op (void) +metrics_get_current_op (metrics_t *metrics) { - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; - return metrics->op; } static void -op_metrics_init (context_t *ctx, op_metrics_t *metrics, metrics_op_t op) +op_metrics_init (metrics_info_t *info, op_metrics_t *metrics, metrics_op_t op) { - metrics_info_t *info = ctx->metrics->info; unsigned i, j; metrics->op = op; @@ -359,9 +348,8 @@ op_metrics_init (context_t *ctx, op_metrics_t *metrics, metrics_op_t op) } static op_metrics_t * -ctx_get_op_metrics (context_t *ctx, metrics_op_t op) +_get_op_metrics (metrics_t *metrics, metrics_op_t op) { - metrics_t *metrics = ctx->metrics; unsigned i; if (op >= metrics->num_op_metrics) @@ -369,7 +357,7 @@ ctx_get_op_metrics (context_t *ctx, metrics_op_t op) metrics->op_metrics = realloc (metrics->op_metrics, (op + 1) * sizeof (op_metrics_t)); for (i = metrics->num_op_metrics; i < op + 1; i++) - op_metrics_init (ctx, &metrics->op_metrics[i], i); + op_metrics_init (metrics->info, &metrics->op_metrics[i], i); metrics->num_op_metrics = op + 1; } @@ -378,7 +366,8 @@ ctx_get_op_metrics (context_t *ctx, metrics_op_t op) } static void -accumulate_program_metrics (metrics_op_t op, GLuint *result, GLuint size) +accumulate_program_metrics (metrics_t *metrics, metrics_op_t op, + GLuint *result, GLuint size) { #define CONSUME(var) \ if (p + sizeof(var) > ((unsigned char *) result) + size) \ @@ -390,9 +379,8 @@ accumulate_program_metrics (metrics_op_t op, GLuint *result, GLuint size) (var) = *((typeof(var) *) p); \ p += sizeof(var); - context_t *ctx = context_get_current (); - metrics_info_t *info = ctx->metrics->info; - op_metrics_t *metrics = ctx_get_op_metrics (ctx, op); + metrics_info_t *info = metrics->info; + op_metrics_t *op_metrics = _get_op_metrics (metrics, op); unsigned char *p = (unsigned char *) result; while (p < ((unsigned char *) result) + size) @@ -446,19 +434,18 @@ accumulate_program_metrics (metrics_op_t op, GLuint *result, GLuint size) break; } - metrics->counters[group_index][counter_index] += value; + op_metrics->counters[group_index][counter_index] += value; } } static void -accumulate_program_time (metrics_op_t op, unsigned time_ns) +accumulate_program_time (metrics_t *metrics, metrics_op_t op, unsigned time_ns) { - context_t *ctx = context_get_current (); - op_metrics_t *metrics; + op_metrics_t *op_metrics; - metrics = ctx_get_op_metrics (ctx, op); + op_metrics = _get_op_metrics (metrics, op); - metrics->time_ns += time_ns; + op_metrics->time_ns += time_ns; } typedef struct per_stage_metrics @@ -497,12 +484,12 @@ _is_shader_stage_counter (metrics_info_t *info, } static void -print_per_stage_metrics (context_t *ctx, +print_per_stage_metrics (metrics_t *metrics, per_stage_metrics_t *per_stage, double total) { - metrics_info_t *info = ctx->metrics->info; - op_metrics_t *metric = per_stage->metrics; + metrics_info_t *info = metrics->info; + op_metrics_t *op_metrics = per_stage->metrics; metrics_group_info_t *group; const char *op_string; unsigned group_index, counter; @@ -512,12 +499,12 @@ print_per_stage_metrics (context_t *ctx, if (per_stage->time_ns == 0.0) return; - op_string = metrics_op_string (metric->op); + op_string = metrics_op_string (op_metrics->op); printf ("%21s", op_string); - if (metric->op >= METRICS_OP_SHADER) { - printf (" %3d", metric->op - METRICS_OP_SHADER); + if (op_metrics->op >= METRICS_OP_SHADER) { + printf (" %3d", op_metrics->op - METRICS_OP_SHADER); } else { printf (" "); @@ -554,7 +541,7 @@ print_per_stage_metrics (context_t *ctx, if (_is_shader_stage_counter (info, group_index, counter)) continue; - value = metric->counters[group_index][counter]; + value = op_metrics->counters[group_index][counter]; if (value == 0.0) continue; printf ("%s: %.2f ", group->counter_names[counter], @@ -579,10 +566,8 @@ time_compare(const void *in_a, const void *in_b, void *arg unused) } static void -print_program_metrics (void) +print_program_metrics (metrics_t *metrics) { - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; metrics_info_t *info = metrics->info; unsigned num_shader_stages = info->num_shader_stages; per_stage_metrics_t *sorted, *per_stage; @@ -670,76 +655,14 @@ print_program_metrics (void) time_compare, metrics->op_metrics); for (i = 0; i < num_sorted; i++) - print_per_stage_metrics (ctx, &sorted[i], total_time); + print_per_stage_metrics (metrics, &sorted[i], total_time); free (sorted); } -/* Called at program exit. - * - * This is similar to metrics_info_fini, but only frees any used - * memory. Notably, it does not call any OpenGL functions, (since the - * OpenGL context no longer exists at program exit). - */ -static void -metrics_exit (void) -{ - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; - metrics_info_t *info = metrics->info; - unsigned i, j; - timer_query_t *timer, *timer_next; - monitor_t *monitor, *monitor_next; - - if (verbose) - printf ("fips: terminating\n"); - - if (! info->initialized) - return; - - for (timer = metrics->timer_head; - timer; - timer = timer_next) - { - timer_next = timer->next; - free (timer); - } - - for (monitor = metrics->monitor_head; - monitor; - monitor = monitor_next) - { - monitor_next = monitor->next; - free (monitor); - } - - for (i = 0; i < info->num_groups; i++) { - metrics_group_info_t *group = &info->groups[i]; - - for (j = 0; j < group->num_counters; i++) - free (group->counter_names[j]); - - free (group->counter_types); - free (group->counter_names); - free (group->counter_ids); - - free (group->name); - } - - free (info->groups); - - for (i = 0; i < info->num_shader_stages; i++) - free (info->stages[i].name); - - free (info->stages); -} - void -metrics_collect_available (void) +metrics_collect_available (metrics_t *metrics) { - context_t *ctx = context_get_current (); - metrics_t *metrics = ctx->metrics; - /* Consume all timer queries that are ready. */ timer_query_t *timer = metrics->timer_head; @@ -754,7 +677,7 @@ metrics_collect_available (void) glGetQueryObjectuiv (timer->id, GL_QUERY_RESULT, &elapsed); - accumulate_program_time (timer->op, elapsed); + accumulate_program_time (metrics, timer->op, elapsed); metrics->timer_head = timer->next; if (metrics->timer_head == NULL) @@ -792,7 +715,7 @@ metrics_collect_available (void) result_size, result, &bytes_written); - accumulate_program_metrics (monitor->op, result, result_size); + accumulate_program_metrics (metrics, monitor->op, result, result_size); free (result); @@ -812,14 +735,13 @@ metrics_collect_available (void) void -metrics_end_frame (void) +metrics_end_frame (metrics_t *metrics) { static int initialized = 0; static struct timeval tv_start, tv_now; if (! initialized) { gettimeofday (&tv_start, NULL); - atexit (metrics_exit); if (getenv ("FIPS_VERBOSE")) verbose = 1; initialized = 1; @@ -827,7 +749,7 @@ metrics_end_frame (void) frames++; - metrics_collect_available (); + metrics_collect_available (metrics); if (frames % 15 == 0) { double fps; @@ -839,6 +761,6 @@ metrics_end_frame (void) printf("FPS: %.3f\n", fps); - print_program_metrics (); + print_program_metrics (metrics); } } diff --git a/metrics.h b/metrics.h index 230d668..69afafb 100644 --- a/metrics.h +++ b/metrics.h @@ -81,11 +81,11 @@ metrics_destroy (metrics_t *metrics); * current program (as set with metrics_set_current_program). */ void -metrics_counter_start (void); +metrics_counter_start (metrics_t *metrics); /* Stop accumulating GPU time (stops the most-recently started counter) */ void -metrics_counter_stop (void); +metrics_counter_stop (metrics_t *metrics); /* Set a metrics_op_t value to indicate what kind of operation is * being performed. @@ -100,13 +100,13 @@ metrics_counter_stop (void); * METRICS_OP_SHADER + shader_program_number to this function. */ void -metrics_set_current_op (metrics_op_t op); +metrics_set_current_op (metrics_t *metrics, metrics_op_t op); /* Return the current metrics_op_t value, (the value most-recently-set * with a call to metrics_set_current_op). */ metrics_op_t -metrics_get_current_op (void); +metrics_get_current_op (metrics_t *metrics); /* Should be called at the end of every function wrapper for a * function that ends a frame, (glXSwapBuffers and similar). @@ -115,7 +115,7 @@ metrics_get_current_op (void); * generate a timing report, then emits that report. */ void -metrics_end_frame (void); +metrics_end_frame (metrics_t *metrics); /* Process outstanding metrics requests, accumulating results. * @@ -129,6 +129,6 @@ metrics_end_frame (void); * measured. */ void -metrics_collect_available (void); +metrics_collect_available (metrics_t *metrics); #endif -- 2.43.0