Re: batching gets and searching

[Colm Smyth <Colm Smyth ireland sun com>]

>To: Colm Smyth <Colm Smyth ireland sun com>
>Cc: gconf-list gnome org
>Subject: Re: batching gets and searching
>From: Havoc Pennington <hp redhat com>
>
>Colm Smyth <Colm Smyth ireland sun com> writes:   
>>   in which case it is limited to getting keys from a single directory.
>>
>
>Note that we already have this function, called gconf_all_entries()
>(perhaps a bit oddly). Well, I guess you can't get a subset of keys in
>a directory using that function.

You can't get a subset and the keys have to be in a single directory - 
that's quite limiting. An app could potentially fetch all keys in a single
get operation with a batch-get API.

>(I'm seriously wondering why I named all the GConfEngine methods
>gconf_* instead of gconf_engine_*, should probably change that.)

Agreed.

>Anyway: also, using GConfClient you can preload a directory into the
>cache if you know you'll be using all the stuff in that directory. 
>In general gconf_client_get() is cheaper than gconf_get() since it
>caches stuff locally.

A cache helps with the current API only if all keys are in a single directory.

>> - int gconf_value_group_get( GConfValue **group, const char **keys, int nkeys 
);
>> 
>>   Same idea, just that the keys are in a separate array and GConfValue 
pointers
>>   are set by the API.
>>
>
>The only reason I see to use arrays rather than lists is to keep
>keys/values associated (i.e. group[i] is the value for keys[i]). 
>If you do that you need to put NULL in for any unset values, so the
>int return value isn't needed.
>
>How about:
>
>GSList* gconf_get_multiple (GConfEngine  *engine,
>                            const gchar **keys,
>                            GError      **err);
>
>where the returned GSList contains GConfEntry with the key set to the
>fully-qualified key?

There's nowhere to return the value; perhaps you meant to return values
in the GSList?

>I don't like the fact that the key is fully qualified here and not for
>gconf_all_entries() though. I think the right solution may be to
>always fully qualify the key in a GConfEntry. I'm not sure how large
>that change is.

I think a fully qualified GConfEntry makes more sense in general; as you
noted above, there is value in keeping the key and value associated.

>> Alternatively, it would be possible to extend the notion of GConfChangeSet
>> to also include gets.
>> 
>
>A ChangeSet is a set of modifications, rather than a set of values, so
>I'd rather not do that. 

Agreed that is the purpose of ChangeSet, but there is value in having
a datatype that represents a set of database operations, which could
include reads. An API based around GConfEntry works too, provided the
key may include it's directory. 

The downside of this arrangement is that it makes reads and writes asymmetric;
a batched read operation uses GSList<GConfEntry> gconf_all_entries() or 
gconf_get_multiple(GConfEntry *) while a batched write will use e.g.
gconf_commit_change_set(GConfChangeSet).

The only value I can see to this asymmetry is that it is more important
that a set of writes contains only a single value for a given key, whereas
it is ok for a set of reads to read the same key more than once.


>> One other API we need when creating a GConf GUI is the ability to search
>> recursively through keys and values. For efficiency, a search must be
>> done on the server (gconfd) side. I suggest:
>> 
>> GConfSearch *gconf_search_new (
>>  const gchar *key_root,       /* directory to search from */
>>  const gchar *search_value,   /* expression to search for */
>
>I would guess a key question is, what is this expression and what does
>it match? (Do we convert int/bool/float keys into a string?)

For the purposes of searching, yes. Given that the purpose of the API
is to enable a searchable GConf GUI, it would be too complex to
allow input of values of each specific type.

>Shouldn't you be able to restrict a search to keys of a particular
>type, for example?
>
>>  int flags, /* GCONF_SEARCH_KEY | GCONF_SEARCH_VALUE | ... */
>
>The convention in G* libs is "GConfSearchFlags flags", for better or
>worse, where GConfSearchFlags is the enum containing the flags.

The individual flags should be an enum, but the flags may be ORed together so
the API argument should be an int.

>>  int depth, /* -1 = unlimited, 0 => no directories, 1 => 1 directory deep, 
... 
>> */
>>  );
>>  GSList *gconf_search_apply ( GConfSearch *search,
>>    int maxkeys /* maximum number of keys to retrieve, < 1 implies limited 
only
>>      by system limits */
>>   );
>
>How would you select a max depth or number of keys? Would this be a
>user-visible option in the GUI? 

Yes to both.

>
>>  GConfEntry *gconf_search_next (GConfSearch *search); /* returns the first or
>>    next value; the state of the search is maintained to ensure a linear
>>    breadth-first search of the tree is carried out */
>
>Hmm, another argument for putting the full key in GConfEntry always.

Yes, I hope this isn't too big a change; GConfEntry appears seldom
in the code so I think this should be easy. The main downside is it
will affect users of the GConf API, but again this should be a minor
change.

>
>>  void gconf_search_destroy (GConfSearch *search); /* destroys all resources
>>    associated with this search, including memory allocated to search struct 
*/
>>    
>> I'd welcome feedback!
>> 
>
>Sounds good, we certainly need these two features. It looks like the
>hard-ish parts are a) finding all instances where the code assumes
>entry->key is a relative name and changing them (or maybe we could put
>a key_is_absolute field in GConfEntry?) and b) implementing search in
>the backends... also we'll need interfaces for these things in the
>backend and in the GConf.idl layer.
>
>(The most annoying thing about working on GConf is what I call
>"lasagna code" distinct from spaghetti code - there are all these
>near-identical layers that have to be kept in sync - GConfClient,
>GConfEngine, GConf.idl, gconfd CORBA implementation, gconf-sources.h,
>backend interface, backend implementation...)

This discourages altering the type or number of arguments to an API,
but at least the GConfEntry change just alters the expected content
of a struct's member, so this should not impact on the "middleware"
layers. It will impact on GConfClient (caching code) and backend
implementations of gconf_all_entries().

By the way, I love your phrase "lasagna code"!

Colm.

>Havoc
>
>
>
>_______________________________________________
>Gconf-list mailing list
>Gconf-list gnome org
>http://mail.gnome.org/mailman/listinfo/gconf-list