Hi,

attached is a path that introduces logic to disallow creation of a fish path that is actually slower than appropriate reference fish.

Suggested Changelog entry:

* babl/babl-fish-path.c (get_conversion_path), (babl_fish_path), (test_create), (init_path_instrumentation), (destroy_path_instrumentation), (get_path_instrumentation): Improved fish path instrumentation during the search for a new one to optimize for speed. Added logic to disallow creation of a fish path that is actually slower than appropriate reference fish.

* babl/babl-util.[ch] (babl_process_cost): New function for unified timing formula for babl processings.

* babl/babl-conversion.c (babl_conversion_error): Make use of the new babl_process_cost function.

* extensions/gimp-8bit.c: Added a function declaration to prevent compiler warning.

Regards, Jan

Index: extensions/gimp-8bit.c =================================================================== --- extensions/gimp-8bit.c (revision 304) +++ extensions/gimp-8bit.c (working copy) @@ -296,6 +296,7 @@ conv_g8_gamma_2_2_rgbaF_linear (unsigned return samples;
}

+int init (void);

int
init (void)
Index: babl/babl-fish-path.c
=================================================================== --- babl/babl-fish-path.c (revision 304) +++ babl/babl-fish-path.c (working copy) @@ -23,7 +23,17 @@
#define BABL_MAX_COST_VALUE 2000000
static double
-get_conversion_path_error (BablList *path); +init_path_instrumentation (Babl *fmt_source, + Babl *fmt_destination); +
+static void
+destroy_path_instrumentation (void); +
+static void
+get_path_instrumentation (BablList *path, + double *path_cost, + double *ref_cost, + double *path_error);
static long
process_conversion_path (BablList *path, @@ -118,28 +128,32 @@ get_conversion_path (Babl *current_forma {
/* We have found a candidate path, let's * see about it's properties */ - double temp_cost = 0.0;
- double temp_error = 1.0;
+ double path_cost = 0.0;
+ double ref_cost = 0.0;
+ double path_error = 1.0;
int i;

for (i = 0; i < babl_list_size (current_path); i++) {
- temp_error *= (1.0 + babl_conversion_error ((BablConversion *) current_path->items[i])); - temp_cost += babl_conversion_cost ((BablConversion *) current_path->items[i]); + path_error *= (1.0 + babl_conversion_error ((BablConversion *) current_path->items[i])); }

- if (temp_cost < fish_path->fish_path.cost && - temp_error - 1.0 fish_path.cost = path_cost; + fish_path->fish.error = path_error; + babl_list_copy (current_path, fish_path->fish_path.conversion_list); + }
}
}
else
@@ -163,11 +177,10 @@ get_conversion_path (Babl *current_forma if (!next_format->format.visited) {
/* next_format is not in the current path, we can pay a visit */ - babl_list_insert_last (current_path, next_conversion); - get_conversion_path (next_format, current_length + 1, max_length); - babl_list_remove_last (current_path); - }
-
+ babl_list_insert_last (current_path, next_conversion); + get_conversion_path (next_format, current_length + 1, max_length); + babl_list_remove_last (current_path); + }
}

/* Remove the current format from current path */ @@ -228,6 +241,7 @@ babl_fish_path (const Babl *source, to_format = (Babl *) destination;
get_conversion_path ((Babl *) source, 0, max_path_length ()); + destroy_path_instrumentation ();

babl_list_destroy (current_path);
@@ -328,25 +342,21 @@ process_conversion_path (BablList *path, return n;
}

-#define NUM_TEST_PIXELS (128 + 16 + 16) +#define NUM_TEST_PIXELS (256 + 16 + 16)
static double *
test_create (void)
{
static double test[sizeof (double) * NUM_TEST_PIXELS * 4]; - static int flag = 0;
int i, j;

/* There is no need to generate the test * more times ... */
- if (flag)
- return test;
- flag = 1;

srandom (20050728);

/* add 128 pixels in the valid range between 0.0 and 1.0 */ - for (i = 0; i < 128 * 4; i++)
+ for (i = 0; i < 256 * 4; i++)
test [i] = (double) random () / RAND_MAX;
/* add 16 pixels between -1.0 and 0.0 */ @@ -360,38 +370,49 @@ test_create (void) return test;
}

+// FishPath instrumentation
+
+static Babl *fmt_rgba_double = NULL; +static double *test = NULL;
+static void *source;
+static void *destination;
+static void *ref_destination;
+static double *destination_rgba_double; +static double *ref_destination_rgba_double; +static Babl *fish_rgba_to_source; +static Babl *fish_reference;
+static Babl *fish_destination_to_rgba; +static double reference_cost;
+static int init_instrumentation_done = 0; +
static double
-get_conversion_path_error (BablList *path) +init_path_instrumentation (Babl *fmt_source, + Babl *fmt_destination) {
- Babl *fmt_rgba_double = babl_format_new ( - babl_model ("RGBA"),
- babl_type ("double"),
- babl_component ("R"),
- babl_component ("G"),
- babl_component ("B"),
- babl_component ("A"),
- NULL);
-
- double error = 0.0;
-
- Babl *fmt_source = (Babl *) BABL (babl_list_get_first (path))->conversion.source; - Babl *fmt_destination = (Babl *) BABL (babl_list_get_last (path))->conversion.destination; -
- double *test;
- void *source;
- void *destination;
- double *destination_rgba_double;
- void *ref_destination;
- double *ref_destination_rgba_double; -
- Babl *fish_rgba_to_source = babl_fish_reference (fmt_rgba_double, - fmt_source); - Babl *fish_reference = babl_fish_reference (fmt_source, - fmt_destination); - Babl *fish_destination_to_rgba = babl_fish_reference (fmt_destination, - fmt_rgba_double); + long ticks_start = 0;
+ long ticks_end = 0;
+
+ if (!fmt_rgba_double)
+ {
+ fmt_rgba_double = babl_format_new ( + babl_model ("RGBA"),
+ babl_type ("double"),
+ babl_component ("R"),
+ babl_component ("G"),
+ babl_component ("B"),
+ babl_component ("A"),
+ NULL);
+ }

- test = test_create ();
+ if (!test)
+ test = test_create ();
+
+ fish_rgba_to_source = babl_fish_reference (fmt_rgba_double, + fmt_source); + fish_reference = babl_fish_reference (fmt_source, + fmt_destination); + fish_destination_to_rgba = babl_fish_reference (fmt_destination, + fmt_rgba_double);
source = babl_calloc (NUM_TEST_PIXELS, fmt_source->format.bytes_per_pixel); @@ -409,23 +430,68 @@ get_conversion_path_error (BablList *pat test, source, NUM_TEST_PIXELS);
/* calculate the reference buffer of how it should be */ + ticks_start = babl_ticks ();
babl_process (fish_reference,
source, ref_destination, NUM_TEST_PIXELS); + ticks_end = babl_ticks ();
+ reference_cost = babl_process_cost (ticks_start, ticks_end); +
+ /* transform the reference destination buffer to RGBA */ + babl_process (fish_destination_to_rgba, + ref_destination, ref_destination_rgba_double, NUM_TEST_PIXELS); +}
+
+static void
+destroy_path_instrumentation (void) +{
+ if (init_instrumentation_done)
+ {
+ babl_free (source);
+ babl_free (destination);
+ babl_free (destination_rgba_double); + babl_free (ref_destination);
+ babl_free (ref_destination_rgba_double); +
+ /* nulify the flag for potential new search */ + init_instrumentation_done = 0; + }
+}
+
+static void
+get_path_instrumentation (BablList *path, + double *path_cost, + double *ref_cost, + double *path_error) +{
+ long ticks_start = 0;
+ long ticks_end = 0;
+
+ if (!init_instrumentation_done)
+ {
+ /* this initialization can be done only once since the + * source and destination formats do not change during + * the search */
+ Babl *fmt_source = (Babl *) BABL (babl_list_get_first (path))->conversion.source; + Babl *fmt_destination = (Babl *) BABL (babl_list_get_last (path))->conversion.destination; + init_path_instrumentation (fmt_source, fmt_destination); + init_instrumentation_done = 1; + }

/* calculate this path's view of what the result should be */ + ticks_start = babl_ticks ();
process_conversion_path (path, source, destination, NUM_TEST_PIXELS); + ticks_end = babl_ticks ();
+ *path_cost = babl_process_cost (ticks_start, ticks_end);
/* transform the reference and the actual destination buffers to RGBA * for comparison with each other */
babl_process (fish_destination_to_rgba, - ref_destination, ref_destination_rgba_double, NUM_TEST_PIXELS); - babl_process (fish_destination_to_rgba, destination, destination_rgba_double, NUM_TEST_PIXELS);
- error = babl_rel_avg_error (destination_rgba_double, - ref_destination_rgba_double, - NUM_TEST_PIXELS * 4); + *path_error = babl_rel_avg_error (destination_rgba_double, + ref_destination_rgba_double, + NUM_TEST_PIXELS * 4);
fish_rgba_to_source->fish.processings--; fish_reference->fish.processings--; @@ -435,11 +501,5 @@ get_conversion_path_error (BablList *pat fish_reference->fish.pixels -= NUM_TEST_PIXELS; fish_destination_to_rgba->fish.pixels -= 2 * NUM_TEST_PIXELS;
- babl_free (source);
- babl_free (destination);
- babl_free (destination_rgba_double); - babl_free (ref_destination);
- babl_free (ref_destination_rgba_double); -
- return error;
+ *ref_cost = reference_cost;
}
Index: babl/babl-util.c
=================================================================== --- babl/babl-util.c (revision 304)
+++ babl/babl-util.c (working copy)
@@ -46,6 +46,13 @@ babl_ticks (void) return usecs (measure_time) - usecs (start_time); }

+long
+babl_process_cost (long ticks_start, + long ticks_end)
+{
+ return (ticks_end - ticks_start) * 10 + 1; +}
+
double
babl_rel_avg_error (double *imgA,
double *imgB,
Index: babl/babl-util.h
=================================================================== --- babl/babl-util.h (revision 304)
+++ babl/babl-util.h (working copy)
@@ -22,6 +22,10 @@
long
babl_ticks (void);

+long
+babl_process_cost (long ticks_start, + long ticks_end); +
double
babl_rel_avg_error (double *imgA,
double *imgB,
Index: babl/babl-conversion.c
=================================================================== --- babl/babl-conversion.c (revision 304) +++ babl/babl-conversion.c (working copy) @@ -550,7 +550,7 @@ babl_conversion_error (BablConversion *c babl_free (test);

conversion->error = error;
- conversion->cost = (ticks_end - ticks_start) * 10 + 1; + conversion->cost = babl_process_cost (ticks_start, ticks_end);
return error;
}

On 00:51, Mon 14 Apr 08, Øyvind Kolås wrote:

One issue in babl's current design is that other activity on the system at the same time as measuring is performed can skew results quite a lot for a given conversion, not sure if there is a good way to deal with this at runtime though.

Hi,

yes, the gettimeofday function is indeed not the best profiling method. I have put some though into this and came up with several alternatives how to tackle this problem.

The first possibility is to replace gettimeofday by system function that accounts only for time of the given process, such as getrusage. Although the getrusage function provides way to measure process time in microseconds, the real resolution is far from 1 microsecond. Using getrusage would mean that the runtime profiling would have to take approx. 100x longer than in the current state to get reasonable results for the fastest conversions. And that is far from ideal.

The second possibility is to profile during the compilation, which would allow for much longer and precise profiling. This would generate some profiling data in a form of a C file and would compile into babl in the second compile pass. This might work in a situation when the compiling machine is the target machine, however, the results can be of course skewed while this is not the case. Maybe the biggest drawback would be the fact that this contradicts the dynamic paradigm of the library.

The third possibility is again to profile longer using getrusage or similar function, this time once per user. The profiling results would be output e.g. to ~/.babl/prof.data and would be reloaded next time. Once new .so with conversion would be found or new babl version installed, the profiling would be done again.

So these are the alternatives I came up with. If you prefer one of these or have a better idea, I can investigate it further.

Regards, Jan