Hi,

Here is an another patch I came up with while playing with BABL code. It concerns constructors of the BABL classes. The original code prepares the whole instance of a class before trying to insert it into the BABL database. The insertion might fail in case there is an instance of the same name already present in the database. This is a bit wasteful, so the patch tries to test the preexistent instance as soon as possible.

It showed up that this brought a considerable speedup. It speeds babl_fish_path_dhtml by up to 9%. Speedup of everyday BABL usage won't be as significant, but measurable nonetheless.

I sneaked two minor typo fixups into the patch as well.

Regards, Jan

Index: babl/babl-model.c =================================================================== --- babl/babl-model.c (revision 297) +++ babl/babl-model.c (working copy) @@ -161,14 +161,24 @@ babl_model_new (void *first_argument,
va_end (varg);

- babl = model_new (create_name (name, components, component), id, components, component); + name = create_name (name, components, component);
- {
- Babl *ret = babl_db_insert (db, babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+ babl = babl_db_exist (db, id, name); + if (babl)
+ {
+ /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
+ babl = model_new (name, id, components, component); +
+ /* Since there is not an already registered instance by the required + * id/name, inserting newly created class into database. + */
+ babl_db_insert (db, babl);
+ return babl;
}


Index: babl/babl-fish-path.c
=================================================================== --- babl/babl-fish-path.c (revision 297) +++ babl/babl-fish-path.c (working copy) @@ -284,6 +284,15 @@ babl_fish_path (const Babl *source, char *name = create_name (source, destination, 1); BablConversion *temp_chain[BABL_HARD_MAX_PATH_LENGTH];
+ babl = babl_db_exist_by_name (babl_fish_db (), name); + if (babl)
+ {
+ /* There is an instance already registered by the required name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
babl_assert (BABL_IS_BABL (source)); babl_assert (BABL_IS_BABL (destination));
@@ -329,12 +338,11 @@ babl_fish_path (const Babl *source, return NULL;
}

- {
- Babl *ret = babl_db_insert (babl_fish_db (), babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+ /* Since there is not an already registered instance by the required + * name, inserting newly created class into database. + */
+ babl_db_insert (babl_fish_db (), babl); + return babl;
}

static long
Index: babl/babl-format.c
=================================================================== --- babl/babl-format.c (revision 297) +++ babl/babl-format.c (working copy) @@ -45,24 +45,25 @@ format_new (const char *name, {
Babl *babl;

- {
- int i;
- /* i is desintation position */ - for (i = 0; i < model->components; i++) - {
- int j;
-
- for (j = 0; j < components; j++) - {
- if (component[j] == model->component[i]) - goto component_found; - }
- babl_fatal ("matching source component for %s in model %s not found", - model->component[i]->instance.name, model->instance.name); -component_found:
- ;
- }
- }
+ /* i is desintation position */
+ int i, j, component_found = 0;
+ for (i = 0; i < model->components; i++) + {
+ for (j = 0; j < components; j++) + {
+ if (component[j] == model->component[i]) + {
+ component_found = 1;
+ break;
+ }
+ }
+ if (!component_found)
+ {
+ component_found = 0;
+ babl_fatal ("matching source component for %s in model %s not found", + model->component[i]->instance.name, model->instance.name); + }
+ }

/* allocate all memory in one chunk */ babl = babl_malloc (sizeof (BablFormat) + @@ -295,17 +296,28 @@ babl_format_new (void *first_arg,
va_end (varg);

- babl = format_new (name ? name : create_name (model, components, component, type), + if (!name)
+ name = create_name (model, components, component, type); +
+ babl = babl_db_exist (db, id, name); + if (babl)
+ {
+ /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
+ babl = format_new (name,
id,
planar, components, model, component, sampling, type);
- {
- Babl *ret = babl_db_insert (db, babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+ /* Since there is not an already registered instance by the required + * id/name, inserting newly created class into database. + */
+ babl_db_insert (db, babl);
+ return babl;
}

int
Index: babl/babl-component.c
=================================================================== --- babl/babl-component.c (revision 297) +++ babl/babl-component.c (working copy) @@ -109,14 +109,22 @@ babl_component_new (void *first_arg,
va_end (varg);

+ babl = babl_db_exist (db, id, first_arg); + if (babl)
+ {
+ /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
babl = component_new (first_arg, id, luma, chroma, alpha);
- {
- Babl *ret = babl_db_insert (db, babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+ /* Since there is not an already registered instance by the required + * id/name, inserting newly created class into database. + */
+ babl_db_insert (db, babl);
+ return babl;
}

BABL_CLASS_TEMPLATE (component)
Index: babl/babl-extension.c
=================================================================== --- babl/babl-extension.c (revision 297) +++ babl/babl-extension.c (working copy) @@ -184,7 +184,7 @@ babl_extension_load (const char *path) init = dlsym (dl_handle, "init"); if (!init)
{
- babl_log ("\n\tint babl_extension_init() function not found in extenstion '%s'", path); + babl_log ("\n\tint babl_extension_init() function not found in extension '%s'", path); dlclose (dl_handle);
return load_failed (babl);
}
@@ -202,7 +202,8 @@ babl_extension_load (const char *path) return load_failed (babl);
}

- if (babl_db_insert (db, babl) == babl) + babl_db_insert (db, babl);
+ if (babl == babl_db_exist_by_name (db, path)) {
babl_set_extender (NULL);
return babl;
Index: babl/babl-fish-simple.c
=================================================================== --- babl/babl-fish-simple.c (revision 297) +++ babl/babl-fish-simple.c (working copy) @@ -34,6 +34,15 @@ babl_fish_simple (BablConversion *conver
name = create_name (conversion);

+ babl = babl_db_exist_by_name (babl_fish_db (), name); + if (babl)
+ {
+ /* There is an instance already registered by the required name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
babl = babl_malloc (sizeof (BablFishSimple) + strlen (name) + 1); babl->class_type = BABL_FISH_SIMPLE; @@ -50,10 +59,9 @@ babl_fish_simple (BablConversion *conver reference, and babl fish reference only requests clean conversions */
- {
- Babl *ret = babl_db_insert (babl_fish_db (), babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+ /* Since there is not an already registered instance by the required + * name, inserting newly created class into database. + */
+ babl_db_insert (babl_fish_db (), babl); + return babl;
}
Index: babl/babl-type.c
=================================================================== --- babl/babl-type.c (revision 297)
+++ babl/babl-type.c (working copy)
@@ -128,14 +128,22 @@ babl_type_new (void *first_arg,
va_end (varg);

+ babl = babl_db_exist (db, id, first_arg); + if (babl)
+ {
+ /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
babl = type_new (first_arg, id, bits);
- {
- Babl *ret = babl_db_insert (db, babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+ /* Since there is not an already registered instance by the required + * id/name, inserting newly created class into database. + */
+ babl_db_insert (db, babl);
+ return babl;
}


Index: babl/babl-conversion.c
=================================================================== --- babl/babl-conversion.c (revision 297) +++ babl/babl-conversion.c (working copy) @@ -79,7 +79,7 @@ conversion_new (const char *name, }
else if (planar)
{
- babl_fatal ("planar conversions not supported for %ssupported", + babl_fatal ("planar conversions not supported for %s", babl_class_name (source->class_type)); }
break;
@@ -257,18 +257,26 @@ babl_conversion_new (void *first_arg, {
type = BABL_CONVERSION_PLANAR; }
- babl = conversion_new (create_name (source, destination, type), - id, source, destination, linear, plane, planar);
- {
- Babl *ret = babl_db_insert (db, babl); - if (ret != babl)
- babl_free (babl);
- else
- babl_add_ptr_to_list ((void ***) ((Babl *) &(source->type.from)), babl); -
- return ret;
- }
+ char * name = create_name (source, destination, type); +
+ babl = babl_db_exist (db, id, name); + if (babl)
+ {
+ /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
+ babl = conversion_new (name, id, source, destination, linear, plane, planar); +
+ /* Since there is not an already registered instance by the required + * id/name, inserting newly created class into database. + */
+ babl_db_insert (db, babl);
+ babl_add_ptr_to_list ((void ***) ((Babl *) &(source->type.from)), babl); + return babl;
}

static long
Index: babl/babl-db.c
=================================================================== --- babl/babl-db.c (revision 297)
+++ babl/babl-db.c (working copy)
@@ -94,12 +94,6 @@ Babl *
babl_db_insert (BablDb *db,
Babl *item)
{
-
- Babl *found = babl_db_exist (db, item->instance.id, item->instance.name); -
- if (found)
- return found;
-
if (item->instance.id)
babl_hash_table_insert (db->id_hash, item); babl_hash_table_insert (db->name_hash, item); Index: babl/babl-fish-reference.c
=================================================================== --- babl/babl-fish-reference.c (revision 297) +++ babl/babl-fish-reference.c (working copy) @@ -51,6 +51,15 @@ babl_fish_reference (const Babl *source, Babl *babl = NULL;
char *name = create_name (source, destination, 1);
+ babl = babl_db_exist_by_name (babl_fish_db (), name); + if (babl)
+ {
+ /* There is an instance already registered by the required name, + * returning the preexistent one instead. + */
+ return babl;
+ }
+
babl_assert (BABL_IS_BABL (source)); babl_assert (BABL_IS_BABL (destination));
@@ -71,12 +80,12 @@ babl_fish_reference (const Babl *source, babl->fish.error = 0.0; /* assuming the provided reference conversions for types and models are as exact as possible */ - {
- Babl *ret = babl_db_insert (babl_fish_db (), babl); - if (ret != babl)
- babl_free (babl);
- return ret;
- }
+
+ /* Since there is not an already registered instance by the required + * name, inserting newly created class into database. + */
+ babl_db_insert (babl_fish_db (), babl); + return babl;
}

Hi

On 20:04, Sat 29 Mar 08, Sven Neumann wrote:

BTW, wasn't there some additional changes needed after your last patch? If I remember correctly, some lists still need to be ported to the new list API. Is that correct?

That is correct. I will definitely port the rest of the code to the new API, I'm just contemplating the best course of action. There are other related changes I would want to make and I was thinking I'd make them all at once.

Regards, Jan