Re: [gnome-db] gda_holder_set_value_static_str

[Massimo Cora' <maxcvs email it>]

Hi Vivien,

here it is a possible patch. I added only the function
gda_holder_take_static_value (), in this way it's possible to use both
strings or G_TYPE_* values.
Putting all the values static in my test program I could gain an average
performance of 20ms quicker than the normal method for 100 queries.
User should then care to free the const GValue* passed to the holder and
he must also be sure to let the holder has a valid GValue at query time.

Please review it and tell me what you think,

thanks and regards,
Massimo



Massimo Cora' wrote:
> 
> Vivien Malerba wrote:
>> 2008/8/25 Massimo Cora' <maxcvs email it>:
>>> Hi Vivien,
>>>
>>>
>>> while trying to optimize the speed of some queries I came on
>>> g_value_set_static_string (), which is good and faster than its brother
>>> g_value_set_str ().
>>> I was wondering if it was possibile to add a
>>> gda_holder_set_value_static_str ()?
>> This can be done, even if it would be usefull only for GdaHolder with
>> a G_TYPE_STRING value.
>>
>>> Another question, always in that fashion:
>>> gda_holder_take_value () takes as input a *value that is freed when the
>>> holder is re-set. That method returns TRUE if value has been set.
>>> I was thinking about this speed improvement:
>>> create a gda_holder_take_value_do_not_free () [well, the name is
>>> unlucky...] that when is re-set will return the stored value instead of
>>> freeing it. In this manner it's possible to reuse that GValue without a
>>> g_free () and g_value_new_*().
>> Could you provide a patch for this?
> 
> sure I'll try to come out with a patch ASAP.
> 
> regards,
> Massimo
> 
> 
> _______________________________________________
> gnome-db-list mailing list
> gnome-db-list gnome org
> http://mail.gnome.org/mailman/listinfo/gnome-db-list
> 

Index: libgda/gda-holder.c
===================================================================
--- libgda/gda-holder.c	(revision 3210)
+++ libgda/gda-holder.c	(working copy)
@@ -93,6 +93,7 @@
 	
 	gboolean         invalid_forced;
 	gboolean         valid;
+	gboolean         is_freeable;
 
 	GValue           *value;
 	GValue          *default_value; /* CAN be either NULL or of any type */
@@ -267,6 +268,7 @@
 	holder->priv->invalid_forced = FALSE;
 	holder->priv->valid = TRUE;
 	holder->priv->default_forced = FALSE;
+	holder->priv->is_freeable = TRUE;
 	holder->priv->value = NULL;
 	holder->priv->default_value = NULL;
 
@@ -339,6 +341,7 @@
 		/* direct settings */
 		holder->priv->invalid_forced = orig->priv->invalid_forced;
 		holder->priv->valid = orig->priv->valid;
+		holder->priv->is_freeable = orig->priv->is_freeable;
 		holder->priv->default_forced = orig->priv->default_forced;	
 		if (orig->priv->value)
 			holder->priv->value = gda_value_copy (orig->priv->value);
@@ -466,8 +469,9 @@
 
 		holder->priv->g_type = G_TYPE_INVALID;
 
-		if (holder->priv->value) {
-			gda_value_free (holder->priv->value);
+		if (holder->priv->value) {			
+			if (holder->priv->is_freeable)
+				gda_value_free (holder->priv->value);
 			holder->priv->value = NULL;
 		}
 
@@ -792,23 +796,23 @@
 
 	if (!value || !g_ascii_strcasecmp (value, "NULL")) 
                 return gda_holder_set_value (holder, NULL, error);
-        else {
-                GValue *gdaval = NULL;
+    else {
+		GValue *gdaval = NULL;
 
 		if (!dh)
 			dh = gda_get_default_handler (holder->priv->g_type);
-                if (dh)
-                        gdaval = gda_data_handler_get_value_from_str (dh, value, holder->priv->g_type);
+		if (dh)
+        	gdaval = gda_data_handler_get_value_from_str (dh, value, holder->priv->g_type);
 
-                if (gdaval)
+		if (gdaval)
 			return real_gda_holder_set_value (holder, gdaval, FALSE, error);
-                else {
+        else {
 			g_set_error (error, GDA_HOLDER_ERROR, GDA_HOLDER_STRING_CONVERSION_ERROR,
 				     _("Unable to convert string to '%s' type"), 
 				     gda_g_type_to_string (holder->priv->g_type));
-                        return FALSE;
+        	return FALSE;
 		}
-        }
+	}
 }
 
 /**
@@ -858,6 +862,12 @@
 	gboolean was_valid = gda_holder_is_valid (holder);
 #endif
 
+	/* if the value has been set with gda_holder_take_static_value () you'll be able
+	 * to change the value only with another call to real_gda_holder_set_value 
+	 */
+	if (!holder->priv->is_freeable)
+		return FALSE;		
+		
 	/* holder will be changed? */
 	newnull = !value || gda_value_is_null (value);
 	current_val = gda_holder_get_value (holder);
@@ -960,7 +970,149 @@
 	return newvalid;
 }
 
+static GValue *
+real_gda_holder_set_const_value (GdaHolder *holder, const GValue *value, GError **error)
+{
+	gboolean changed = TRUE;
+	gboolean newvalid;
+	const GValue *current_val;
+	GValue *value_to_return = NULL;
+	gboolean newnull;
+#define DEBUG_HOLDER
+#undef DEBUG_HOLDER
+
+#ifdef DEBUG_HOLDER
+	gboolean was_valid = gda_holder_is_valid (holder);
+#endif
+
+	/* holder will be changed? */
+	newnull = !value || gda_value_is_null (value);
+	current_val = gda_holder_get_value (holder);
+	if (current_val == value)
+		changed = FALSE;
+	else if ((!current_val || gda_value_is_null ((GValue *)current_val)) && newnull)
+		changed = FALSE;
+	else if (value && current_val &&
+		 (G_VALUE_TYPE (value) == G_VALUE_TYPE ((GValue *)current_val)))
+		changed = gda_value_differ (value, (GValue *)current_val);
+		
+	/* holder's validity */
+	newvalid = TRUE;
+	if (newnull && holder->priv->not_null) {
+		g_set_error (error, GDA_HOLDER_ERROR, GDA_HOLDER_VALUE_NULL_ERROR,
+			     _("Holder does not allow NULL values"));
+		newvalid = FALSE;
+		changed = TRUE;
+	}
+	else if (!newnull && (G_VALUE_TYPE (value) != holder->priv->g_type)) {
+		g_set_error (error, GDA_HOLDER_ERROR, GDA_HOLDER_VALUE_TYPE_ERROR,
+			     _("Wrong value type: expected type '%s' when value's type is '%s'"),
+			     gda_g_type_to_string (holder->priv->g_type),
+			     gda_g_type_to_string (G_VALUE_TYPE (value)));
+		newvalid = FALSE;
+		changed = TRUE;
+	}
+
+#ifdef DEBUG_HOLDER
+	g_print ("Changed holder %p (%s): value %s --> %s \t(type %d -> %d) VALID: %d->%d CHANGED: %d\n", 
+		 holder, holder->priv->id,
+		 current_val ? gda_value_stringify ((GValue *)current_val) : "_NULL_",
+		 value ? gda_value_stringify ((value)) : "_NULL_",
+		 current_val ? G_VALUE_TYPE ((GValue *)current_val) : 0,
+		 value ? G_VALUE_TYPE (value) : 0, 
+		 was_valid, newvalid, changed);
+#endif
+
+	/* end of procedure if the value has not been changed, after calculating the holder's validity */
+	if (!changed) {
+		holder->priv->invalid_forced = FALSE;
+		return NULL;
+	}
+
+	/* check if we are allowed to change value */
+	GError *lerror = NULL;
+	g_signal_emit (holder, gda_holder_signals[VALIDATE_CHANGE], 0, value, &lerror);
+	if (lerror) {
+		/* change refused by signal callback */
+		g_propagate_error (error, lerror);
+		return NULL;
+	}
+
+	/* new valid status */
+	holder->priv->invalid_forced = FALSE;
+	holder->priv->valid = newvalid;
+	holder->priv->is_freeable = FALSE;
+
+	/* check is the new value is the default one */
+	holder->priv->default_forced = FALSE;
+	if (holder->priv->default_value) {
+		if ((G_VALUE_TYPE (holder->priv->default_value) == GDA_TYPE_NULL) && newnull)
+			holder->priv->default_forced = TRUE;
+		else if ((G_VALUE_TYPE (holder->priv->default_value) == holder->priv->g_type) &&
+			 value && (G_VALUE_TYPE (value) == holder->priv->g_type))
+			holder->priv->default_forced = !gda_value_compare (holder->priv->default_value, value);
+	}
+
+	/* real setting of the value */
+	if (holder->priv->full_bind) {
+#ifdef DEBUG_HOLDER
+		g_print ("Holder %p is alias of holder %p => propagating changes to holder %p\n",
+			 holder, holder->priv->full_bind, holder->priv->full_bind);
+#endif
+		return real_gda_holder_set_const_value (holder->priv->full_bind, value, error);
+	}
+	else {
+		if (holder->priv->value) {
+			value_to_return = holder->priv->value;
+			holder->priv->value = NULL;
+		}
+
+		if (value) {
+			if (newvalid) {
+				holder->priv->value = value;
+			}
+		}
+
+		g_signal_emit (holder, gda_holder_signals[CHANGED], 0);
+	}
+
+	return value_to_return;
+}
+
 /**
+ * gda_holder_take_static_value
+ * @holder: a #GdaHolder object
+ * @value: a const value to set the holder to
+ * @error: a place to store errors, or %NULL
+ *
+ * Sets the const value within the holder. If @holder is an alias for another
+ * holder, then the value is also set for that other holder.
+ *
+ * The value will not be freed, and user should take care of it, either for its
+ * freeing or for its correct value at the moment of query.
+ * 
+ * If the value is not different from the one already contained within @holder,
+ * then @holder is not chaged and no signal is emitted.
+ *
+ * Note1: if @holder can't accept the @value value, then this method returns NULL, and @holder will be left
+ * in an invalid state.
+ *
+ * Note2: before the change is accepted by @holder, the "validate-change" signal will be emitted (the value
+ * of which can prevent the change from happening) which can be connected to to have a greater control
+ * of which values @holder can have, or implement some business rules.
+ *
+ * Returns: TRUE if value has been set
+ */
+GValue *
+gda_holder_take_static_value (GdaHolder *holder, const GValue *value, GError **error)
+{
+	g_return_val_if_fail (GDA_IS_HOLDER (holder), FALSE);
+	g_return_val_if_fail (holder->priv, FALSE);
+
+	return real_gda_holder_set_const_value (holder, (GValue*) value, error);
+}
+
+/**
  * gda_holder_force_invalid
  * @holder: a #GdaHolder object
  *
@@ -986,7 +1138,8 @@
 	holder->priv->valid = FALSE;
 	
 	if (holder->priv->value) {
-		gda_value_free (holder->priv->value);
+		if (holder->priv->is_freeable)
+			gda_value_free (holder->priv->value);
 		holder->priv->value = NULL;
 	}
 
@@ -1048,7 +1201,8 @@
 		holder->priv->default_forced = TRUE;
 		holder->priv->invalid_forced = FALSE;
 		if (holder->priv->value) {
-			gda_value_free (holder->priv->value);
+			if (holder->priv->is_freeable)
+				gda_value_free (holder->priv->value);
 			holder->priv->value = NULL;
 		}
 	}
@@ -1361,7 +1515,8 @@
 
 		/* get rid of the internal holder's value */
 		if (holder->priv->value) {
-			gda_value_free (holder->priv->value);
+			if (holder->priv->is_freeable)
+				gda_value_free (holder->priv->value);
 			holder->priv->value = NULL;
 		}
 
Index: libgda/gda-holder.h
===================================================================
--- libgda/gda-holder.h	(revision 3210)
+++ libgda/gda-holder.h	(working copy)
@@ -75,6 +75,7 @@
 gchar              *gda_holder_get_value_str           (GdaHolder *holder, GdaDataHandler *dh);
 gboolean            gda_holder_set_value               (GdaHolder *holder, const GValue *value, GError **error);
 gboolean            gda_holder_take_value              (GdaHolder *holder, GValue *value, GError **error);
+GValue             *gda_holder_take_static_value       (GdaHolder *holder, const GValue *value, GError **error);
 gboolean            gda_holder_set_value_str           (GdaHolder *holder, GdaDataHandler *dh, const gchar *value, GError **error);
 
 const GValue       *gda_holder_get_default_value       (GdaHolder *holder);
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 3211)
+++ ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2008-09-18  Massimo Cora'  <maxcvs email it>
+
+	* libgda/gda-holder.c (gda_holder_init), (gda_holder_copy),
+	(gda_holder_dispose), (real_gda_holder_set_value),
+	(real_gda_holder_set_const_value), (gda_holder_take_static_value),
+	(gda_holder_force_invalid), (gda_holder_set_value_to_default),
+	(gda_holder_set_full_bind):
+	* libgda/gda-holder.h:
+	added function gda_holder_take_static_value () to permit a quicker use of GValues
+	without allocation/deallocation. On my tests I can gain 20ms on 100 queries.
+
 2008-09-18  Vivien Malerba <malerba gnome-db org>
 
 	* libgda/gda-data-model.c: make gda_data_model_dump() work whith data models which