registry: use binary format instead of XML for cache

registry: use binary format instead of XML for cache

Currently the registry cache is in the XML format, that may cause
unnecessary parsing overhead on ibus-daemon startup.  This patch
changes the format to GVariant based binary representation.

BUG=
R=takao.fujiwara1@gmail.com

Committed: 632a5c1

[Daiki Ueno, 2013-06-03 09:33:41]

Tested on Fedora 19 Beta.  The cache size has been considerably reduced:

-rw-r--r-- 1 ueno ueno 115959 Jun  3 18:31 registry
-rw-r--r-- 1 ueno ueno 273082 May 22 17:31 registry.xml

[Peng, 2013-06-03 14:56:13]

https://codereview.appspot.com/9963043/diff/1/bus/registry.c
File bus/registry.c (right):

https://codereview.appspot.com/9963043/diff/1/bus/registry.c#newcode42
bus/registry.c:42: IBusObject parent;
How about make BusRegistry serializable? I am also thinking if it is necessary
to public it as ibus API. Is there some use cases of it?

https://codereview.appspot.com/9963043/diff/1/bus/registry.c#newcode264
bus/registry.c:264: variant = g_variant_new_from_data (G_VARIANT_TYPE
("a{sav}"),
Maybe we should put version information into the register?

https://codereview.appspot.com/9963043/diff/1/bus/registry.c#newcode267
bus/registry.c:267: TRUE,
trusted? Could you please explain what does it mean?

[Daiki Ueno, 2013-06-04 03:29:00]

https://codereview.appspot.com/9963043/diff/1/bus/registry.c
File bus/registry.c (right):

https://codereview.appspot.com/9963043/diff/1/bus/registry.c#newcode42
bus/registry.c:42: IBusObject parent;
On 2013/06/03 14:56:13, Peng wrote:
> How about make BusRegistry serializable?

Done.

> I am also thinking if it is necessary
> to public it as ibus API. Is there some use cases of it?

I have no idea, but if we plan to add a command to generate the cache, public
API would be useful to implement it in Vala.

https://codereview.appspot.com/9963043/diff/1/bus/registry.c#newcode264
bus/registry.c:264: variant = g_variant_new_from_data (G_VARIANT_TYPE
("a{sav}"),
On 2013/06/03 14:56:13, Peng wrote:
> Maybe we should put version information into the register?

Done.

https://codereview.appspot.com/9963043/diff/1/bus/registry.c#newcode267
bus/registry.c:267: TRUE,
On 2013/06/03 14:56:13, Peng wrote:
> trusted? Could you please explain what does it mean?

Oops, sorry, my mistake.

[Daiki Ueno, 2013-07-11 07:02:49]

So, I've rebased against the git master, after IBusRegistry addition.  Please
review it if you have time.

[Peng, 2013-07-11 16:10:53]

+ Fujieara-san

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c
File src/ibusregistry.c (right):

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode31
src/ibusregistry.c:31: #define IBUS_REGISTRY_CACHE_MAGIC "IBUS"
Suggest using a 4 bytes integer. So you can use == to compare it

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode334
src/ibusregistry.c:334: version = (p[0] << 24) | (p[1] << 16) | (p[2] << 8) |
p[3];
maybe use version = g_ntohl(*(guint32*)p)?

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode428
src/ibusregistry.c:428: *p++ = (IBUS_REGISTRY_CACHE_VERSION >> 24) & 0xFF;
g_htonl

[Peng, 2013-07-11 16:11:40]

On 2013/07/11 16:10:53, Peng wrote:
> + Fujieara-san
> 
> https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c
> File src/ibusregistry.c (right):
> 
> https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode31
> src/ibusregistry.c:31: #define IBUS_REGISTRY_CACHE_MAGIC "IBUS"
> Suggest using a 4 bytes integer. So you can use == to compare it
> 
>
https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode334
> src/ibusregistry.c:334: version = (p[0] << 24) | (p[1] << 16) | (p[2] << 8) |
> p[3];
> maybe use version = g_ntohl(*(guint32*)p)?
> 
>
https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode428
> src/ibusregistry.c:428: *p++ = (IBUS_REGISTRY_CACHE_VERSION >> 24) & 0xFF;
> g_htonl

lgtm with some comments. And please wait Fujiwara-san's feedback too thanks.

[Daiki Ueno, 2013-07-12 03:11:33]

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c
File src/ibusregistry.c (right):

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode31
src/ibusregistry.c:31: #define IBUS_REGISTRY_CACHE_MAGIC "IBUS"
On 2013/07/11 16:10:53, Peng wrote:
> Suggest using a 4 bytes integer. So you can use == to compare it

Done.

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode334
src/ibusregistry.c:334: version = (p[0] << 24) | (p[1] << 16) | (p[2] << 8) |
p[3];
On 2013/07/11 16:10:53, Peng wrote:
> maybe use version = g_ntohl(*(guint32*)p)?

Done.  I used GUINT32_FROM_BE instead of g_ntohl to be more explicit.

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode428
src/ibusregistry.c:428: *p++ = (IBUS_REGISTRY_CACHE_VERSION >> 24) & 0xFF;
On 2013/07/11 16:10:53, Peng wrote:
> g_htonl

Done.

[fujiwara, 2013-07-12 06:18:24]

lgtm with comments.

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c
File src/ibusregistry.c (right):

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode321
src/ibusregistry.c:321: if (!g_file_get_contents (filename, &contents, &length,
NULL))
I think it's useful to output GError from g_file_get_contents() and
g_file_set_contents().

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode327
src/ibusregistry.c:327: if (length < IBUS_REGISTRY_CACHE_MAGIC_SIZE + 4 ||
bus_registry_load_cache_file() uses 4 and ibus_registry_save_cache_file()
uses sizeof(gunit32).
Is it better to uniform with a way?

Also p += 4 below.

[Daiki Ueno, 2013-07-12 09:32:14]

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c
File src/ibusregistry.c (right):

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode321
src/ibusregistry.c:321: if (!g_file_get_contents (filename, &contents, &length,
NULL))
On 2013/07/12 06:18:25, fujiwara wrote:
> I think it's useful to output GError from g_file_get_contents() and
> g_file_set_contents().

Done.

https://codereview.appspot.com/9963043/diff/16001/src/ibusregistry.c#newcode327
src/ibusregistry.c:327: if (length < IBUS_REGISTRY_CACHE_MAGIC_SIZE + 4 ||
On 2013/07/12 06:18:25, fujiwara wrote:
> bus_registry_load_cache_file() uses 4 and ibus_registry_save_cache_file()
> uses sizeof(gunit32).
> Is it better to uniform with a way?
> 
> Also p += 4 below.

Done.

[Daiki Ueno, 2013-07-12 09:40:01]

Committed patchset #8 manually as r632a5c1 (presubmit successful).