All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: "Geneviève Bastien" <gbastien+lttng@versatic.net>
Cc: lttng-dev@lists.lttng.org
Subject: Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm
Date: Sun, 27 Apr 2014 08:34:02 +0000 (UTC)	[thread overview]
Message-ID: <2087397408.8083.1398587642565.JavaMail.zimbra__29938.8989843012$1398587700$gmane$org@efficios.com> (raw)
In-Reply-To: <53458452.8030905@versatic.net>

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Wednesday, April 9, 2014 7:33:06 PM
> Subject: Re: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm
> 
> On 04/09/2014 11:42 AM, Mathieu Desnoyers wrote:
> > ----- Original Message -----
> >> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> >> To: lttng-dev@lists.lttng.org
> >> Sent: Wednesday, March 26, 2014 10:47:09 AM
> >> Subject: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global
> >> structures	for ust-comm
> >>
> >> It flatten the global types so that global types used by other global
> >> types
> > flatten -> flattens
> >
> >> all belong to the event at serialization. This avoids recursive calls to
> >> the
> >> send and receive functions and simplifies error handling (no recursive
> >> frees).
> > Is the structure and enum part of the event, or declared outside of
> > the event scope ?
> They are named structures so declared outside the scope of the event.
> But they are sent only if one event needs them.
> >
> > Is there a downside to this approach ? For instance, if the same
> > structure is used in many events, do we have to send the structure info
> > many times ?
> Sending each structure (and enum) once would be enough, we could keep a
> hash table of global types that were sent, like we do before dumping the
> metadata of those global types, and that would avoid sending them many
> times. But it is not what is being done right now. We just send all the
> global types, but they are added to the metadata only once.

Repeating the integers, strings, etc on the "wire" (unix socket) for
every field between UST and sessiond was not so bad, since they were
small enough descriptions.

However, I figure that enumerations, structures and variant types will
grow to be much larger than their basic types counterparts.

Therefore, sending the global type separately, and then sending a
reference to that type (a string) when encountering the event field
using this global type seems like a more scalable solution to me.

Thanks,

Mathieu

> 
> Thanks,
> Geneviève
> >
> > Thanks,
> >
> > Mathieu
> >
> >> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
> >> ---
> >>   liblttng-ust-comm/lttng-ust-comm.c | 170
> >>   +++++++++++++++++++++++++++++++++----
> >>   liblttng-ust-ctl/ustctl.c          |  26 ++++++
> >>   2 files changed, 179 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
> >> b/liblttng-ust-comm/lttng-ust-comm.c
> >> index bc22130..90274c4 100644
> >> --- a/liblttng-ust-comm/lttng-ust-comm.c
> >> +++ b/liblttng-ust-comm/lttng-ust-comm.c
> >> @@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types
> >> *uatype,
> >>   	}
> >>   	case atype_array:
> >>   	case atype_sequence:
> >> +	case atype_structure:
> >>   	default:
> >>   		return -EINVAL;
> >>   	}
> >> @@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut, const
> >> struct lttng_type *lt)
> >>   		ut->atype = ustctl_atype_sequence;
> >>   		break;
> >>   	}
> >> +	case atype_structure:
> >> +	{
> >> +		strncpy(ut->u.structure.name, lt->u.structure.name,
> >> +						LTTNG_UST_SYM_NAME_LEN);
> >> +		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> >> +		ut->atype = ustctl_atype_structure;
> >> +		break;
> >> +	}
> >>   	default:
> >>   		return -EINVAL;
> >>   	}
> >> @@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
> >>   }
> >>   
> >>   static
> >> -int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> >> -		struct ustctl_global_type_decl **ustctl_global_type,
> >> +int serialize_structure(struct ustctl_structure *ustruct,
> >> +		const struct lttng_structure *lstruct)
> >> +{
> >> +	int ret;
> >> +
> >> +	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
> >> +	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> >> +
> >> +	/* Serialize the fields */
> >> +	if (lstruct->nr_fields > 0) {
> >> +		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
> >> +				lstruct->nr_fields,	lstruct->fields);
> >> +		if (ret) {
> >> +			return ret;
> >> +		}
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static size_t get_child_global_type_count(size_t nr_global_type_decl,
> >> +		const struct lttng_global_type_decl *lttng_global_type)
> >> +{
> >> +	size_t total_global_types = 0;
> >> +	int i;
> >> +	const struct lttng_global_type_decl *lg;
> >> +
> >> +	for (i = 0; i < nr_global_type_decl; i++) {
> >> +		lg = &lttng_global_type[i];
> >> +
> >> +		if (lg->nowrite)
> >> +			continue;
> >> +
> >> +		/* Count the global types in children as well */
> >> +		switch (lg->mtype) {
> >> +		case mtype_structure:
> >> +		{
> >> +			const struct lttng_structure *ls;
> >> +
> >> +			ls = lg->u.ctf_structure;
> >> +			/* Add the number of global types it contains and the count of its own
> >> children */
> >> +			if (ls->nr_global_type_decl > 0) {
> >> +				total_global_types += ls->nr_global_type_decl;
> >> +				total_global_types +=
> >> get_child_global_type_count(ls->nr_global_type_decl,
> >> +								ls->global_type_decl);
> >> +			}
> >> +			break;
> >> +		}
> >> +		case mtype_enum:
> >> +		default:
> >> +			break;
> >> +		}
> >> +	}
> >> +
> >> +	return total_global_types;
> >> +}
> >> +
> >> +/*
> >> + * When this method is called, index is the next index to write to and
> >> the
> >> + * ustctl_global_type structure is already initialized and will not
> >> overflow.
> >> + *
> >> + * The global type declarations are flattened at serialization to avoid
> >> + * needing to do recursive frees on error.
> >> + */
> >> +static int _serialize_global_type_decl(size_t *index,
> >> +		struct ustctl_global_type_decl *global_type_decl,
> >>   		size_t nr_global_type_decl,
> >>   		const struct lttng_global_type_decl *lttng_global_type)
> >>   {
> >> -	struct ustctl_global_type_decl *global_type_decl;
> >>   	int i, ret;
> >> -	size_t nr_write_global_type_decl = 0;
> >> -
> >> -	global_type_decl = zmalloc(nr_global_type_decl
> >> -					* sizeof(*global_type_decl));
> >> -	if (!global_type_decl)
> >> -		return -ENOMEM;
> >>   
> >>   	for (i = 0; i < nr_global_type_decl; i++) {
> >>   		struct ustctl_global_type_decl *f;
> >>   		const struct lttng_global_type_decl *lf;
> >>   
> >> -		f = &global_type_decl[nr_write_global_type_decl];
> >> +		f = &global_type_decl[*index];
> >>   		lf = &lttng_global_type[i];
> >>   
> >>   		/* skip 'nowrite' fields */
> >> @@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t
> >> *_nr_write_global_type_decl,
> >>   			le = lf->u.ctf_enum;
> >>   			ret = serialize_enum(ue, le);
> >>   			if (ret)
> >> -				goto error;
> >> +				return ret;
> >>   
> >>   			f->mtype = ustctl_mtype_enum;
> >>   			break;
> >>   		}
> >> +		case mtype_structure:
> >> +		{
> >> +			struct ustctl_structure *us;
> >> +			const struct lttng_structure *ls;
> >> +
> >> +			/* Serialize children global types first */
> >> +			ls = lf->u.ctf_structure;
> >> +			ret = _serialize_global_type_decl(index,
> >> +					global_type_decl,
> >> +					ls->nr_global_type_decl,
> >> +					ls->global_type_decl);
> >> +			if (ret)
> >> +				return ret;
> >> +
> >> +			/* Reinitialize f since the index may have changed */
> >> +			f = &global_type_decl[*index];
> >> +			us = &f->u.ctf_structure;
> >> +
> >> +			ret = serialize_structure(us, ls);
> >> +			if (ret)
> >> +				return ret;
> >> +
> >> +			f->mtype = ustctl_mtype_structure;
> >> +			break;
> >> +		}
> >>   		default:
> >> -			ret = -EINVAL;
> >> -			goto error;
> >> +			return -EINVAL;
> >>   		}
> >>   
> >> -		nr_write_global_type_decl++;
> >> +		*index = *index + 1;
> >>   	}
> >> +	return 0;
> >> +}
> >> +
> >> +static
> >> +int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> >> +		struct ustctl_global_type_decl **ustctl_global_type,
> >> +		size_t nr_global_type_decl,
> >> +		const struct lttng_global_type_decl *lttng_global_type)
> >> +{
> >> +	struct ustctl_global_type_decl *global_type_decl;
> >> +	int i, ret;
> >> +	size_t nr_write_global_type_decl = 0;
> >> +	size_t nr_child_global_type_cnt =
> >> get_child_global_type_count(nr_global_type_decl,
> >> +							lttng_global_type);
> >> +
> >> +	global_type_decl = zmalloc((nr_global_type_decl +
> >> nr_child_global_type_cnt)
> >> +					* sizeof(*global_type_decl));
> >> +	if (!global_type_decl)
> >> +		return -ENOMEM;
> >> +
> >> +	ret = _serialize_global_type_decl(&nr_write_global_type_decl,
> >> global_type_decl,
> >> +			nr_global_type_decl, lttng_global_type);
> >> +	if (ret)
> >> +		goto error;
> >>   
> >>   	*_nr_write_global_type_decl = nr_write_global_type_decl;
> >>   	*ustctl_global_type = global_type_decl;
> >> @@ -954,6 +1068,9 @@ error:
> >>   		case ustctl_mtype_enum:
> >>   			free(m->u.ctf_enum.entries);
> >>   			break;
> >> +		case ustctl_mtype_structure:
> >> +			free(m->u.ctf_structure.fields);
> >> +			break;
> >>   		default:
> >>   			break;
> >>   		}
> >> @@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
> >>   				}
> >>   				break;
> >>   			}
> >> +			case ustctl_mtype_structure:
> >> +			{
> >> +				int field_len = one_global_type->u.ctf_structure.nr_fields *
> >> sizeof(*one_global_type->u.ctf_structure.fields);
> >> +
> >> +				/* Send the fields */
> >> +				DBG("Sending fields for global type structure %s.\n",
> >> +						one_global_type->u.ctf_structure.name);
> >> +				len = ustcomm_send_unix_sock(sock,
> >> one_global_type->u.ctf_structure.fields, field_len);
> >> +				free(one_global_type->u.ctf_structure.fields);
> >> +				one_global_type->u.ctf_structure.fields = NULL;
> >> +				if (len > 0 && len != field_len) {
> >> +					goto error_global_type;
> >> +				}
> >> +				if (len < 0) {
> >> +					goto error_global_type;
> >> +				}
> >> +				break;
> >> +			}
> >>   			default:
> >>   				break;
> >>   			}
> >>   		}
> >> -		free(global_type_decl);
> >>   
> >> -	} else {
> >> -		free(global_type_decl);
> >>   	}
> >> +	free(global_type_decl);
> >>   
> >>   	/* receive reply */
> >>   	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
> >> @@ -1212,6 +1345,9 @@ error_global_type:
> >>   		case ustctl_mtype_enum:
> >>   			free(one_global_type->u.ctf_enum.entries);
> >>   			break;
> >> +		case ustctl_mtype_structure:
> >> +			free(one_global_type->u.ctf_structure.fields);
> >> +			break;
> >>   		default:
> >>   			break;
> >>   		}
> >> diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
> >> index a181e62..3100f1b 100644
> >> --- a/liblttng-ust-ctl/ustctl.c
> >> +++ b/liblttng-ust-ctl/ustctl.c
> >> @@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
> >>   				}
> >>   				break;
> >>   			}
> >> +			case ustctl_mtype_structure:
> >> +			{
> >> +				int entry_len = one_global_type->u.ctf_structure.nr_fields *
> >> sizeof(*one_global_type->u.ctf_structure.fields);
> >> +				/* Receive the entries */
> >> +				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
> >> +				if (!one_global_type->u.ctf_structure.fields) {
> >> +					len = -ENOMEM;
> >> +					goto global_type_error;
> >> +				}
> >> +				len = ustcomm_recv_unix_sock(sock,
> >> +						one_global_type->u.ctf_structure.fields,
> >> +						entry_len);
> >> +				DBG("Received fields for struct %s.\n",
> >> one_global_type->u.ctf_structure.name);
> >> +				if (len > 0 && len != entry_len) {
> >> +					len = -EIO;
> >> +					goto global_type_error;
> >> +				}
> >> +				if (len == 0) {
> >> +					len = -EPIPE;
> >> +					goto global_type_error;
> >> +				}
> >> +				if (len < 0) {
> >> +					goto global_type_error;
> >> +				}
> >> +				break;
> >> +			}
> >>   			default:
> >>   				break;
> >>   			}
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> lttng-dev mailing list
> >> lttng-dev@lists.lttng.org
> >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >>
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

  parent reply	other threads:[~2014-04-27  8:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1395845232-17669-1-git-send-email-gbastien+lttng@versatic.net>
2014-03-26 14:47 ` [RFC Patch Ust 1/5] Update data structures to support CTF global structures Geneviève Bastien
2014-03-26 14:47 ` [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm Geneviève Bastien
2014-03-26 14:47 ` [RFC Patch Ust 3/5] Add the macros to generate the data structures for CTF global structures Geneviève Bastien
2014-03-26 14:47 ` [RFC Patch Ust 4/5] Update the ctf-global-type example to show the usage of a global structure Geneviève Bastien
2014-03-26 14:47 ` [RFC Patch Ust 5/5] Update the LTTng documentation with CTF global structures Geneviève Bastien
     [not found] ` <1395845232-17669-2-git-send-email-gbastien+lttng@versatic.net>
2014-04-09 15:36   ` [RFC Patch Ust 1/5] Update data structures to support " Mathieu Desnoyers
     [not found]   ` <953381139.1247.1397057797666.JavaMail.zimbra@efficios.com>
2014-04-09 17:29     ` Geneviève Bastien
     [not found]     ` <5345836A.1090509@versatic.net>
2014-04-27 11:24       ` Mathieu Desnoyers
     [not found] ` <1395845232-17669-3-git-send-email-gbastien+lttng@versatic.net>
2014-04-09 15:42   ` [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm Mathieu Desnoyers
     [not found]   ` <224451157.1253.1397058138828.JavaMail.zimbra@efficios.com>
2014-04-09 17:33     ` Geneviève Bastien
     [not found]     ` <53458452.8030905@versatic.net>
2014-04-27  8:34       ` Mathieu Desnoyers [this message]
     [not found]       ` <2087397408.8083.1398587642565.JavaMail.zimbra@efficios.com>
2014-05-01  1:05         ` Geneviève Bastien
     [not found]         ` <53619DDF.5060909@versatic.net>
2014-05-02  9:29           ` Mathieu Desnoyers
2014-05-02 13:40 Thibault, Daniel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='2087397408.8083.1398587642565.JavaMail.zimbra__29938.8989843012$1398587700$gmane$org@efficios.com' \
    --to=mathieu.desnoyers@efficios.com \
    --cc=gbastien+lttng@versatic.net \
    --cc=lttng-dev@lists.lttng.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.