Re: [Cache V2 1/1] grl-net: Use libsoup cache
- From: Víctor M. Jáquez L. <vjaquez igalia com>
- To: grilo-list gnome org
- Subject: Re: [Cache V2 1/1] grl-net: Use libsoup cache
- Date: Fri, 11 Mar 2011 11:27:37 +0100
Comments below
On Fri, Mar 11, 2011 at 10:55:59AM +0100, Juan A. Suarez Romero wrote:
> If libsoup version is newer enough, use the cache feature it provides.
>
> Thus, it will cache the content to speed up network access.
>
> Signed-off-by: Juan A. Suarez Romero <jasuarez igalia com>
> ---
> configure.ac | 4 +
> libs/net/Makefile.am | 6 +
> libs/net/grl-net-wc.c | 384 ++++++++++++++++++++++++++++++++++++++++++++++++-
> libs/net/grl-net-wc.h | 11 ++-
> libs/net/wc-test.c | 2 +-
> 5 files changed, 399 insertions(+), 8 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index c307620..8a91bd2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -117,6 +117,9 @@ PKG_CHECK_MODULES(NET, libsoup-2.4,
> HAVE_LIBSOUP=yes,
> HAVE_LIBSOUP=no)
>
> +PKG_CHECK_MODULES(NETCACHE, libsoup-2.4 >= 2.33.4,
> + HAVE_CACHED_LIBSOUP=yes,
> + HAVE_CACHED_LIBSOUP=no)
> AC_ARG_ENABLE([grl_net],
> AS_HELP_STRING([--enable-grl-net],
> [Enable Grilo Net library (default: auto)]),
> @@ -134,6 +137,7 @@ AC_ARG_ENABLE([grl_net],
> ])
>
> AM_CONDITIONAL(BUILD_GRILO_NET, test "x$HAVE_LIBSOUP" = "xyes")
> +AM_CONDITIONAL(BUILD_GRILO_NET_WITH_CACHE, test "x$HAVE_CACHED_LIBSOUP" = "xyes")
>
> # ----------------------------------------------------------
> # DEBUG SUPPORT
> diff --git a/libs/net/Makefile.am b/libs/net/Makefile.am
> index d7542fa..b43cc9a 100644
> --- a/libs/net/Makefile.am
> +++ b/libs/net/Makefile.am
> @@ -5,6 +5,7 @@
> #
> # Copyright (C) 2010 Igalia S.L. All rights reserved.
>
> +
> lib_LTLIBRARIES = libgrlnet-@GRL_MAJORMINOR@.la
> noinst_PROGRAMS = wc-test
>
> @@ -20,6 +21,11 @@ libgrlnet_@GRL_MAJORMINOR@_la_CFLAGS = \
> $(DEPS_CFLAGS) \
> $(NET_CFLAGS)
>
> +if BUILD_GRILO_NET_WITH_CACHE
> +libgrlnet_@GRL_MAJORMINOR@_la_CFLAGS += \
> + -DLIBSOUP_WITH_CACHE
> +endif
> +
> libgrlnet_@GRL_MAJORMINOR@_la_LIBADD = \
> $(top_builddir)/src/lib@GRL_NAME@.la \
> $(DEPS_LIBS) \
> diff --git a/libs/net/grl-net-wc.c b/libs/net/grl-net-wc.c
> index 0b6253f..4effa78 100644
> --- a/libs/net/grl-net-wc.c
> +++ b/libs/net/grl-net-wc.c
> @@ -4,6 +4,7 @@
> * Contact: Iago Toral Quiroga <itoral igalia com>
> *
> * Authors: Víctor M. Jáquez L. <vjaquez igalia com>
> + * Juan A. Suarez Romero <jasuarez igalia com>
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public License
> @@ -35,8 +36,18 @@
> #include "config.h"
> #endif
>
> +#include <string.h>
> #include <libsoup/soup.h>
>
> +#ifdef LIBSOUP_WITH_CACHE
> +/* Using the cache feature requires to use the unstable API */
> +#define LIBSOUP_USE_UNSTABLE_REQUEST_API
> +#define BUFFER_SIZE (50*1024)
> +#include <libsoup/soup-cache.h>
> +#include <libsoup/soup-requester.h>
> +#include <libsoup/soup-request-http.h>
> +#endif
> +
> #include <grilo.h>
> #include "grl-net-wc.h"
>
> @@ -47,6 +58,9 @@ enum {
> PROP_0,
> PROP_LOG_LEVEL,
> PROP_THROTTLING,
> + PROP_IDENTIFIER,
> + PROP_CACHE,
> + PROP_CACHE_SIZE,
> };
>
> #define GRL_NET_WC_GET_PRIVATE(object) \
> @@ -57,8 +71,16 @@ enum {
> typedef struct _RequestClosure RequestClosure;
>
> struct _GrlNetWcPrivate {
> + gchar *identifier;
> SoupSession *session;
> SoupLoggerLogLevel log_level;
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + SoupRequester *requester;
> + SoupCache *cache;
> + gboolean use_cache;
> + guint cache_size;
> + gchar *previous_data;
> +#endif
> guint throttling;
> GTimeVal last_request;
> GQueue *pending; /* closure queue for delayed requests */
> @@ -72,6 +94,15 @@ struct _RequestClosure {
> guint source_id;
> };
>
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> +typedef struct {
> + SoupRequest *request;
> + gchar *buffer;
> + gsize length;
> + gsize offset;
> +} RequestResult;
> +#endif
> +
> GQuark
> grl_net_wc_error_quark (void)
> {
> @@ -90,6 +121,10 @@ static void grl_net_wc_get_property (GObject *object,
> GValue *value,
> GParamSpec *pspec);
>
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> +static void cache_down (GrlNetWc *self);
> +#endif
> +
> static void
> grl_net_wc_class_init (GrlNetWcClass *klass)
> {
> @@ -130,6 +165,49 @@ grl_net_wc_class_init (GrlNetWcClass *klass)
> 0, G_MAXUINT, 0,
> G_PARAM_READWRITE |
> G_PARAM_STATIC_STRINGS));
> + /**
> + * GrlNetWc::identifier
> + *
> + * An unique identifier for GrlNet. This identifier is used when cache is
> + * supported. Cached content in a subdirectory named "identifier".
> + */
> + g_object_class_install_property (g_klass,
> + PROP_IDENTIFIER,
> + g_param_spec_string ("identifier",
> + "GrlNet identifier",
> + "Unique identifier for GrlNet",
> + NULL,
> + G_PARAM_READWRITE |
> + G_PARAM_CONSTRUCT_ONLY |
> +
> G_PARAM_STATIC_STRINGS));
Why should we demand to the API user a cache identifier? I guess we could
figure out one with out messing the user.
> + /**
> + * GrlNetWc::cache
> + *
> + * %TRUE if cache must be used. %FALSE otherwise.
> + */
> + g_object_class_install_property (g_klass,
> + PROP_CACHE,
> + g_param_spec_boolean ("cache",
> + "Use cache",
> + "Use cache",
> + TRUE,
> + G_PARAM_READWRITE |
> + G_PARAM_CONSTRUCT |
> + G_PARAM_STATIC_STRINGS));
> + /**
> + * GrlNetWc::cache-size
> + *
> + * Maximum size of cache, in Mb. Default value is 10Mb.
> + */
Why 10Mb? IMO that's too much for xml documents or so. We are not downloading
the media itself, but the metadata. Especially if we are on embedded devices.
> + g_object_class_install_property (g_klass,
> + PROP_CACHE_SIZE,
> + g_param_spec_uint ("cache-size",
> + "Cache size",
> + "Size of cache in Mb",
> + 0, G_MAXUINT, 10,
> + G_PARAM_READWRITE |
> + G_PARAM_CONSTRUCT |
> + G_PARAM_STATIC_STRINGS));
> }
>
> static void
> @@ -141,6 +219,11 @@ grl_net_wc_init (GrlNetWc *wc)
>
> wc->priv->session = soup_session_async_new ();
> wc->priv->pending = g_queue_new ();
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + wc->priv->requester = soup_requester_new();
> + soup_session_add_feature (wc->priv->session,
> + SOUP_SESSION_FEATURE (wc->priv->requester));
> +#endif
> }
>
> static void
> @@ -150,6 +233,12 @@ grl_net_wc_finalize (GObject *object)
>
> wc = GRL_NET_WC (object);
> grl_net_wc_flush_delayed_requests (wc);
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + cache_down (wc);
> + g_free (wc->priv->identifier);
> + g_free (wc->priv->previous_data);
> + g_object_unref (wc->priv->requester);
> +#endif
> g_queue_free (wc->priv->pending);
> g_object_unref (wc->priv->session);
>
> @@ -173,6 +262,15 @@ grl_net_wc_set_property (GObject *object,
> case PROP_THROTTLING:
> grl_net_wc_set_throttling (wc, g_value_get_uint (value));
> break;
> + case PROP_IDENTIFIER:
> + grl_net_wc_set_identifier (wc, g_value_get_string (value));
> + break;
> + case PROP_CACHE:
> + grl_net_wc_set_cache (wc, g_value_get_boolean (value));
> + break;
> + case PROP_CACHE_SIZE:
> + grl_net_wc_set_cache_size (wc, g_value_get_uint (value));
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (wc, propid, pspec);
> }
> @@ -195,6 +293,15 @@ grl_net_wc_get_property (GObject *object,
> case PROP_THROTTLING:
> g_value_set_uint (value, wc->priv->throttling);
> break;
> + case PROP_IDENTIFIER:
> + g_value_set_string (value, wc->priv->identifier);
> + break;
> + case PROP_CACHE:
> + g_value_set_boolean (value, wc->priv->use_cache);
> + break;
> + case PROP_CACHE_SIZE:
> + g_value_set_uint (value, wc->priv->cache_size);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (wc, propid, pspec);
> }
> @@ -261,6 +368,88 @@ parse_error (guint status,
> }
> }
>
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> +static void
> +read_async_cb (GObject *source, GAsyncResult *res, gpointer user_data)
> +{
> + GSimpleAsyncResult *result;
> + RequestResult *rr;
> + SoupMessage *msg;
> + GError *error = NULL;
> + gsize to_read;
> + gssize s;
> +
> + result = G_SIMPLE_ASYNC_RESULT (user_data);
> + rr = g_simple_async_result_get_op_res_gpointer (result);
> +
> + s = g_input_stream_read_finish (G_INPUT_STREAM (source), res, &error);
> + if (s > 0) {
> + /* Continue reading */
> + rr->offset += s;
> + to_read = rr->length - rr->offset;
> + if (!to_read) {
> + /* Buffer is not enough; we need to assign more space */
> + rr->length *= 2;
> + rr->buffer = g_renew (gchar, rr->buffer, rr->length);
> + to_read = rr->length - rr->offset;
> + }
> + g_input_stream_read_async (G_INPUT_STREAM (source), rr->buffer + rr->offset, to_read, G_PRIORITY_DEFAULT, NULL, read_async_cb, user_data);
> + return;
> + }
> +
> + /* Put the end of string */
> + rr->buffer[rr->offset] = '\0';
> +
> + g_input_stream_close (G_INPUT_STREAM (source), NULL, NULL);
> +
> + if (error) {
> + if (error->code == G_IO_ERROR_CANCELLED) {
> + g_simple_async_result_set_error (result, GRL_NET_WC_ERROR,
> + GRL_NET_WC_ERROR_CANCELLED,
> + "Operation was cancelled");
> + } else {
> + g_simple_async_result_set_error (result, GRL_NET_WC_ERROR,
> + GRL_NET_WC_ERROR_UNAVAILABLE,
> + "Data not available");
> + }
> + g_error_free (error);
> +
> + g_simple_async_result_complete (result);
> + return;
> + }
> +
> + msg = soup_request_http_get_message (SOUP_REQUEST_HTTP (rr->request));
> +
> + if (msg) {
> + if (msg->status_code != SOUP_STATUS_OK) {
> + parse_error (msg->status_code,
> + msg->reason_phrase,
> + msg->response_body->data,
> + G_SIMPLE_ASYNC_RESULT (user_data));
> + g_object_unref (msg);
> + }
> + }
> +
> + g_simple_async_result_complete (result);
> +}
> +
> +static void
> +reply_cb (GObject *source, GAsyncResult *res, gpointer user_data)
> +{
> + GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT (user_data);
> + RequestResult *rr = g_simple_async_result_get_op_res_gpointer (result);
> +
> + GInputStream *in = soup_request_send_finish (rr->request, res, NULL);
> + rr->length = soup_request_get_content_length (rr->request) + 1;
> + if (rr->length == 1) {
> + rr->length = BUFFER_SIZE;
Why 50K as a fallback?
> + }
> + rr->buffer = g_new (gchar, rr->length);
> + g_input_stream_read_async (in, rr->buffer, rr->length, G_PRIORITY_DEFAULT, NULL, read_async_cb, user_data);
> +}
> +
> +#else
> +
> static void
> reply_cb (SoupSession *session,
> SoupMessage *msg,
> @@ -299,6 +488,25 @@ message_cancel_cb (GCancellable *cancellable,
> msg, SOUP_STATUS_CANCELLED);
>
> }
> +#endif
> +
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> +static void
> +get_url_now (GrlNetWc *self,
> + const char *url,
> + GAsyncResult *result,
> + GCancellable *cancellable)
> +{
> + RequestResult *rr = g_slice_new0 (RequestResult);
> +
> + rr->request = soup_requester_request (self->priv->requester, url, NULL);
> + g_simple_async_result_set_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (result),
> + rr,
> + NULL);
> + soup_request_send_async (rr->request, cancellable, reply_cb, result);
> +}
> +
> +#else
>
> static void
> get_url_now (GrlNetWc *self,
> @@ -348,6 +556,7 @@ get_url_now (GrlNetWc *self,
> reply_cb,
> result);
> }
> +#endif
>
> static gboolean
> get_url_delayed (gpointer user_data)
> @@ -405,16 +614,49 @@ get_url (GrlNetWc *self,
> }
> }
>
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> +static void
> +cache_down (GrlNetWc *self)
> +{
> + soup_session_remove_feature (self->priv->session,
> + SOUP_SESSION_FEATURE (self->priv->cache));
> + soup_cache_clear (self->priv->cache);
> + g_object_unref (self->priv->cache);
> + self->priv->cache = NULL;
> +}
> +
> +static void
> +cache_up (GrlNetWc *self)
> +{
> + gchar *cache_dir;
> +
> + cache_dir = g_build_filename (g_get_user_cache_dir (),
> + g_get_prgname (),
> + self->priv->identifier,
> + NULL);
> + self->priv->cache = soup_cache_new (cache_dir, SOUP_CACHE_SINGLE_USER);
> + grl_net_wc_set_cache_size (self, self->priv->cache_size);
> + soup_session_add_feature (self->priv->session,
> + SOUP_SESSION_FEATURE (self->priv->cache));
> + g_free (cache_dir);
> +}
> +#endif
> +
> /**
> * grl_net_wc_new:
> + * @identifier: an identifier
Can it be NULL?
> + *
> + * Creates a new #GrlNetWc with unique @identifier.
> *
> * Returns: a new allocated instance of #GrlNetWc. Do g_object_unref() after
> * use it.
> */
> GrlNetWc *
> -grl_net_wc_new (void)
> +grl_net_wc_new (const gchar *identifier)
> {
> - return g_object_new (GRL_TYPE_NET_WC, NULL);
> + return g_object_new (GRL_TYPE_NET_WC,
> + "identifier", identifier,
> + NULL);
> }
>
> /**
> @@ -471,7 +713,6 @@ grl_net_wc_request_finish (GrlNetWc *self,
> GError **error)
> {
> GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT (result);
> - SoupMessage *msg;
> gboolean ret = TRUE;
>
> g_warn_if_fail (g_simple_async_result_get_source_tag (res) ==
> @@ -482,15 +723,40 @@ grl_net_wc_request_finish (GrlNetWc *self,
> goto end_func;
> }
>
> - msg = (SoupMessage *) g_simple_async_result_get_op_res_gpointer (res);
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + RequestResult *rr = g_simple_async_result_get_op_res_gpointer (res);
>
> - if (content != NULL)
> - *content = (gchar *) msg->response_body->data;
> + if (self->priv->previous_data) {
> + g_free (self->priv->previous_data);
> + }
>
> + self->priv->previous_data = rr->buffer;
> +
> + if (content) {
> + *content = self->priv->previous_data;
> + } else {
> + g_free (rr->buffer);
> + }
> +
> + if (length) {
> + *length = rr->offset;
> + }
> +
> +#else
> + SoupMessage *msg = (SoupMessage *) g_simple_async_result_get_op_res_gpointer (res);
> +
> + if (content != NULL)
> + *content = g_strdup ((gchar *) msg->response_body->data);
mmmhh... I wanted to avoid a mem copy, delegating to the user that option if
she wants to maintain the data after a new request is done or unref the wc.
Now that you are forcing the memcopy, we should change all the plugins which
use this.
> if (length != NULL)
> *length = (gsize) msg->response_body->length;
> +#endif
>
> end_func:
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + g_object_unref (rr->request);
> + g_slice_free (RequestResult, rr);
> +#endif
> +
> g_object_unref (res);
> return ret;
> }
> @@ -552,6 +818,112 @@ grl_net_wc_set_throttling (GrlNetWc *self,
> }
>
> /**
> + * grl_net_wc_set_identifier:
> + * @self: a #GrlNetWc instance
> + * @identifier: new identifier
> + *
> + * Changes the identifier for this instance.
> + *
> + * If cache is supported, gets rid of previous cache and create a new one.
> + **/
> +void
> +grl_net_wc_set_identifier (GrlNetWc *self,
> + const gchar *identifier)
> +{
> + g_return_if_fail (GRL_IS_NET_WC (self));
> +
> + if (g_strcmp0 (self->priv->identifier, identifier) == 0) {
> + return;
> + }
> +
> + if (self->priv->identifier) {
> + g_free (self->priv->identifier);
> + }
> +
> + self->priv->identifier = g_strdup (identifier);
> +
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + if (!self->priv->use_cache) {
> + return;
> + }
> +
> + /* Get rid of old cache */
> + if (self->priv->cache) {
> + cache_down (self);
> + }
> +
> + /* Create a new cache */
> + if (self->priv->identifier) {
> + cache_up (self);
> + }
> +#endif
> +}
> +
> +/**
> + * grl_net_wc_set_cache:
> + * @self: a #GrlNetWc instance
> + * @use_cache: if cache must be used or not
> + *
> + * Sets if cache must be used. Note that this will only work if caching is
> + * supporting. If sets %TRUE, a new cache will be created. If sets to %FALSE,
> + * current cache is clean and removed.
> + **/
> +void
> +grl_net_wc_set_cache (GrlNetWc *self,
> + gboolean use_cache)
> +{
> + g_return_if_fail (GRL_IS_NET_WC (self));
> +
> + if (self->priv->use_cache == use_cache) {
> + return;
> + }
> +
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + if (self->priv->cache) {
> + cache_down (self);
> + }
> +
> + self->priv->use_cache = use_cache;
> + if (use_cache && self->priv->identifier) {
> + cache_up (self);
> + }
> +
> +#else
> +
> + GRL_WARNING ("Cache not supported");
> + wc->priv->cache = FALSE;
> +#endif
> +}
> +
> +/**
> + * grl_net_wc_set_cache_size:
> + * @self: a #GrlNetWc instance
> + * @cache_size: size of cache (in Mb)
> + *
> + * Sets the new maximum size of cache, in Megabytes. Default value is 10. Using
> + * 0 means no cache will be done.
> + **/
> +void
> +grl_net_wc_set_cache_size (GrlNetWc *self,
> + guint cache_size)
> +{
> + g_return_if_fail (GRL_IS_NET_WC (self));
> +
> + if (self->priv->cache_size == cache_size) {
> + return;
> + }
> +
> + self->priv->cache_size = cache_size;
> +
> +#ifdef LIBSOUP_USE_UNSTABLE_REQUEST_API
> + if (self->priv->cache) {
> + guint size_in_bytes = cache_size * 1024 * 1024;
> + soup_cache_set_max_size (self->priv->cache, size_in_bytes);
> + }
> +#endif
> +}
> +
> +/**
> * grl_net_wc_flush_delayed_requests:
> * @self: a #GrlNetWc instance
> *
> diff --git a/libs/net/grl-net-wc.h b/libs/net/grl-net-wc.h
> index 71dfd72..5d1ad85 100644
> --- a/libs/net/grl-net-wc.h
> +++ b/libs/net/grl-net-wc.h
> @@ -109,7 +109,7 @@ GType grl_net_wc_get_type (void) G_GNUC_CONST;
>
> GQuark grl_net_wc_error_quark (void) G_GNUC_CONST;
>
> -GrlNetWc *grl_net_wc_new (void);
> +GrlNetWc *grl_net_wc_new (const gchar *identifier);
>
> void grl_net_wc_request_async (GrlNetWc *self,
> const char *uri,
> @@ -129,6 +129,15 @@ void grl_net_wc_set_log_level (GrlNetWc *self,
> void grl_net_wc_set_throttling (GrlNetWc *self,
> guint throttling);
>
> +void grl_net_wc_set_identifier (GrlNetWc *self,
> + const gchar *identifier);
> +
> +void grl_net_wc_set_cache (GrlNetWc *self,
> + gboolean use_cache);
> +
> +void grl_net_wc_set_cache_size (GrlNetWc *self,
> + guint cache_size);
> +
> void grl_net_wc_flush_delayed_requests (GrlNetWc *self);
>
> G_END_DECLS
> diff --git a/libs/net/wc-test.c b/libs/net/wc-test.c
> index 6313c67..8edbdba 100644
> --- a/libs/net/wc-test.c
> +++ b/libs/net/wc-test.c
> @@ -76,7 +76,7 @@ main (int argc, const char **argv)
> if (argc == 2)
> uri = argv[1];
>
> - wc = grl_net_wc_new ();
> + wc = grl_net_wc_new (NULL);
>
> g_object_set (wc, "loglevel", 1, NULL);
>
We shall verify how the apple trailer plugin goes with this type of cache.
vmjl
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
