[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/9] dfa: use a separate data type for grps
From: |
Jim Meyering |
Subject: |
Re: [PATCH 4/9] dfa: use a separate data type for grps |
Date: |
Tue, 03 Jan 2012 10:09:18 +0100 |
Paolo Bonzini wrote:
> * src/dfa.c (leaf_set): New.
> (dfastate): Use leaf_set for grps, as the constraint field is unused.
> ---
> src/dfa.c | 25 +++++++++++++++++--------
> 1 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/src/dfa.c b/src/dfa.c
> index aa87f87..f50fdd0 100644
> --- a/src/dfa.c
> +++ b/src/dfa.c
> @@ -249,6 +249,13 @@ typedef struct
> int nelem; /* Number of elements in this set. */
> } position_set;
>
> +/* Sets of leaves are also stored as arrays. */
> +typedef struct
> +{
> + unsigned int *elems; /* Elements of this position set. */
> + size_t nelem; /* Number of elements in this set. */
> +} leaf_set;
ACK.
However, please mention in the log that this decreases memory usage. E.g.,
dfa: use a more compact type for grps
* src/dfa.c (leaf_set): New type.
(dfastate): Use the smaller type, leaf_set, for grps.
Its prior type contained an unused constraint field.
> /* A state of the dfa consists of a set of positions, some flags,
> and the token value of the lowest-numbered position of the state that
> contains an END token. */
> @@ -2344,7 +2351,7 @@ dfaanalyze (struct dfa *d, int searchflag)
> void
> dfastate (int s, struct dfa *d, int trans[])
> {
> - position_set *grps; /* As many as will ever be needed. */
> + leaf_set *grps; /* As many as will ever be needed. */
> charclass *labels; /* Labels corresponding to the groups. */
> int ngrps = 0; /* Number of groups actually used. */
> position pos; /* Current position being considered. */
> @@ -2466,13 +2473,15 @@ dfastate (int s, struct dfa *d, int trans[])
> copyset(leftovers, labels[ngrps]);
> copyset(intersect, labels[j]);
> MALLOC(grps[ngrps].elems, d->nleaves);
> - copy(&grps[j], &grps[ngrps]);
> + memcpy(grps[ngrps].elems, grps[j].elems,
> + grps[j].nelem * sizeof(unsigned int));
> + grps[ngrps].nelem = grps[j].nelem;
While slightly longer, the following is also slightly safer.
If the type of "elems" ever changes, we won't need to realize the
"sizeof(unsigned int)" above must also be changed to match that new type:
memcpy(grps[ngrps].elems, grps[j].elems,
grps[j].nelem * sizeof(*(grps[j].elems)));
- [PATCH 0/9] dfa refactorings, Paolo Bonzini, 2012/01/03
- [PATCH 1/9] dfa: x2nrealloc starting from a NULL pointer works, Paolo Bonzini, 2012/01/03
- [PATCH 2/9] dfa: remove unnecessary braces, Paolo Bonzini, 2012/01/03
- [PATCH 3/9] dfa: use MALLOC/REALLOC always, Paolo Bonzini, 2012/01/03
- [PATCH 4/9] dfa: use a separate data type for grps, Paolo Bonzini, 2012/01/03
- Re: [PATCH 4/9] dfa: use a separate data type for grps,
Jim Meyering <=
- [PATCH 5/9] dfa: introduce alloc_posset, Paolo Bonzini, 2012/01/03
- [PATCH 6/9] dfa: remove dead assignment, Paolo Bonzini, 2012/01/03
- [PATCH 7/9] dfa: move nalloc to position_set structure, Paolo Bonzini, 2012/01/03
- [PATCH 8/9] dfa: change position_set nelem to size_t, Paolo Bonzini, 2012/01/03
- [PATCH 9/9] dfa: automatically resize position_sets, Paolo Bonzini, 2012/01/03