Re: [PATCH 1/5] builtin/repo: update stats for each object

Justin Tobler [off-list ref] writes:

+		switch (type) {
+		case OBJ_TAG:
+			stats->type_counts.tags++;
+			stats->inflated_sizes.tags += inflated;
+			stats->disk_sizes.tags += disk;
+			break;
+		case OBJ_COMMIT:
+			stats->type_counts.commits++;
+			stats->inflated_sizes.commits += inflated;
+			stats->disk_sizes.commits += disk;
+			break;
+		case OBJ_TREE:
+			stats->type_counts.trees++;
+			stats->inflated_sizes.trees += inflated;
+			stats->disk_sizes.trees += disk;
+			break;
+		case OBJ_BLOB:
+			stats->type_counts.blobs++;
+			stats->inflated_sizes.blobs += inflated;
+			stats->disk_sizes.blobs += disk;
+			break;
+		default:
+			BUG("invalid object type");
+		}
 	}

The repetition above makes me wonder if it might be a better
organization to have

    struct object_stat {       
        struct type_stat {
            size_t count;
            size_t inflated_size;
            size_t disk_size;
	} tag, commit, tree, blob;
	... possibly other members ...
    } *stats;

or even

    struct object_stat {       
        struct type_stat {
            size_t count;
            size_t inflated_size;
            size_t disk_size;
	} t[4];
	... possibly other members ...
    };

and have this part of the code be

	struct type_stat *t;

	if (OBJ_COMMIT <= type && type <= OBJ_TAG)
		t = stats->t[type - 1];
	else
		BUG("invalid object type");

	t->count++;
	t->inflated_size += inflated;
	t->disk_size += disk;

but that is probably only because I am looking at this part of the
code.  Other parts of the code may have good reasons to have the
structure nested the other way around like you have.

On 26/02/03 02:36PM, Junio C Hamano wrote:

Justin Tobler [off-list ref] writes:

quoted

+		switch (type) {
+		case OBJ_TAG:
+			stats->type_counts.tags++;
+			stats->inflated_sizes.tags += inflated;
+			stats->disk_sizes.tags += disk;
+			break;
+		case OBJ_COMMIT:
+			stats->type_counts.commits++;
+			stats->inflated_sizes.commits += inflated;
+			stats->disk_sizes.commits += disk;
+			break;
+		case OBJ_TREE:
+			stats->type_counts.trees++;
+			stats->inflated_sizes.trees += inflated;
+			stats->disk_sizes.trees += disk;
+			break;
+		case OBJ_BLOB:
+			stats->type_counts.blobs++;
+			stats->inflated_sizes.blobs += inflated;
+			stats->disk_sizes.blobs += disk;
+			break;
+		default:
+			BUG("invalid object type");
+		}
 	}

The repetition above makes me wonder if it might be a better
organization to have

    struct object_stat {       
        struct type_stat {
            size_t count;
            size_t inflated_size;
            size_t disk_size;
	} tag, commit, tree, blob;
	... possibly other members ...
    } *stats;

or even

    struct object_stat {       
        struct type_stat {
            size_t count;
            size_t inflated_size;
            size_t disk_size;
	} t[4];
	... possibly other members ...
    };

and have this part of the code be

	struct type_stat *t;

	if (OBJ_COMMIT <= type && type <= OBJ_TAG)
		t = stats->t[type - 1];
	else
		BUG("invalid object type");

	t->count++;
	t->inflated_size += inflated;
	t->disk_size += disk;

but that is probably only because I am looking at this part of the
code.  Other parts of the code may have good reasons to have the
structure nested the other way around like you have.

Good suggestion. Some of the info added in the following commits is
object specific and will need to be handled accordinly, but we could
probably still benefit by structuring the data a bit better. Will
explore in the next version.

-Justin