"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: less malloc/free for got_object_id_queue
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 20 Apr 2022 09:45:05 +0200

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> wrote:
> Inline struct got_object_id in struct got_object_qid.
> This avoids malloc/free calls for every ID on the queue.
> 
> On my system this shaves about 10 seconds from a ca. 3 minute
> run of 'gotadmin pack -a'.

there's a (barely, but still) noticeable speedup when packing got.git =)
(with the hash table plus this diff vs ports' gotadmin)

> Ok?

it's a (mostly) mechanical diff, everything seems fine; ok by me.

> diff b0e81243e25a163bfb4d0b06b6cc6e69ca030bcd fea06cdbdcde1b5b41b419401124589da8390d78
> blob - 0e1d78453eaa209a98462f34d8f8ec152e45e4c4
> blob + b9754c197e68fb0a34968f4b06b7c3540a037ba5
> --- got/got.c
> +++ got/got.c
> @@ -27,6 +27,7 @@
>  #include <limits.h>
>  #include <locale.h>
>  #include <ctype.h>
> +#include <sha1.h>
>  #include <signal.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -3552,7 +3553,7 @@ get_changed_paths(struct got_pathlist_head *paths,
>  	if (qid != NULL) {
>  		struct got_commit_object *pcommit;
>  		err = got_object_open_as_commit(&pcommit, repo,
> -		    qid->id);
> +		    &qid->id);
>  		if (err)
>  			return err;
>  
> @@ -3601,7 +3602,7 @@ print_patch(struct got_commit_object *commit, struct g
>  	qid = STAILQ_FIRST(got_object_commit_get_parent_ids(commit));
>  	if (qid != NULL) {
>  		err = got_object_open_as_commit(&pcommit, repo,
> -		    qid->id);
> +		    &qid->id);
>  		if (err)
>  			return err;
>  	}
> @@ -3860,7 +3861,7 @@ print_commit(struct got_commit_object *commit, struct 
>  		int n = 1;
>  		parent_ids = got_object_commit_get_parent_ids(commit);
>  		STAILQ_FOREACH(qid, parent_ids, entry) {
> -			err = got_object_id_str(&id_str, qid->id);
> +			err = got_object_id_str(&id_str, &qid->id);
>  			if (err)
>  				goto done;
>  			printf("parent %d: %s\n", n++, id_str);
> @@ -4006,7 +4007,8 @@ print_commits(struct got_object_id *root_id, struct go
>  	}
>  	if (reverse_display_order) {
>  		STAILQ_FOREACH(qid, &reversed_commits, entry) {
> -			err = got_object_open_as_commit(&commit, repo, qid->id);
> +			err = got_object_open_as_commit(&commit, repo,
> +			    &qid->id);
>  			if (err)
>  				break;
>  			if (show_changed_paths) {
> @@ -4015,7 +4017,7 @@ print_commits(struct got_object_id *root_id, struct go
>  				if (err)
>  					break;
>  			}
> -			err = print_commit(commit, qid->id, repo, path,
> +			err = print_commit(commit, &qid->id, repo, path,
>  			    show_changed_paths ? &changed_paths : NULL,
>  			    show_patch, diff_context, refs_idmap, NULL);
>  			got_object_commit_close(commit);
> @@ -4708,7 +4710,7 @@ cmd_diff(int argc, char *argv[])
>  			struct got_object_qid *pid;
>  			pids = got_object_commit_get_parent_ids(commit);
>  			pid = STAILQ_FIRST(pids);
> -			ids[0] = got_object_id_dup(pid->id);
> +			ids[0] = got_object_id_dup(&pid->id);
>  			if (ids[0] == NULL) {
>  				error = got_error_from_errno(
>  				    "got_object_id_dup");
> @@ -8530,7 +8532,7 @@ cmd_cherrypick(int argc, char *argv[])
>  		goto done;
>  	pid = STAILQ_FIRST(got_object_commit_get_parent_ids(commit));
>  	memset(&upa, 0, sizeof(upa));
> -	error = got_worktree_merge_files(worktree, pid ? pid->id : NULL,
> +	error = got_worktree_merge_files(worktree, pid ? &pid->id : NULL,
>  	    commit_id, repo, update_progress, &upa, check_cancelled,
>  	    NULL);
>  	if (error != NULL)
> @@ -8632,7 +8634,7 @@ cmd_backout(int argc, char *argv[])
>  	}
>  
>  	memset(&upa, 0, sizeof(upa));
> -	error = got_worktree_merge_files(worktree, commit_id, pid->id,
> +	error = got_worktree_merge_files(worktree, commit_id, &pid->id,
>  	    repo, update_progress, &upa, check_cancelled, NULL);
>  	if (error != NULL)
>  		goto done;
> @@ -9476,7 +9478,7 @@ cmd_rebase(int argc, char *argv[])
>  		error = got_error(GOT_ERR_EMPTY_REBASE);
>  		goto done;
>  	}
> -	error = collect_commits(&commits, commit_id, pid->id,
> +	error = collect_commits(&commits, commit_id, &pid->id,
>  	    yca_id, got_worktree_get_path_prefix(worktree),
>  	    GOT_ERR_REBASE_PATH, repo);
>  	got_object_commit_close(commit);
> @@ -9521,8 +9523,8 @@ cmd_rebase(int argc, char *argv[])
>  	pid = NULL;
>  	STAILQ_FOREACH(qid, &commits, entry) {
>  
> -		commit_id = qid->id;
> -		parent_id = pid ? pid->id : yca_id;
> +		commit_id = &qid->id;
> +		parent_id = pid ? &pid->id : yca_id;
>  		pid = qid;
>  
>  		memset(&upa, 0, sizeof(upa));
> @@ -9536,7 +9538,7 @@ cmd_rebase(int argc, char *argv[])
>  		if (upa.conflicts > 0 || upa.missing > 0 ||
>  		    upa.not_deleted > 0 || upa.unversioned > 0) {
>  			if (upa.conflicts > 0) {
> -				error = show_rebase_merge_conflict(qid->id,
> +				error = show_rebase_merge_conflict(&qid->id,
>  				    repo);
>  				if (error)
>  					goto done;
> @@ -9689,7 +9691,7 @@ histedit_write_commit_list(struct got_object_id_queue 
>  			histedit_cmd = "edit";
>  		else if (fold_only && STAILQ_NEXT(qid, entry) != NULL)
>  			histedit_cmd = "fold";
> -		err = histedit_write_commit(qid->id, histedit_cmd, f, repo);
> +		err = histedit_write_commit(&qid->id, histedit_cmd, f, repo);
>  		if (err)
>  			break;
>  		if (edit_logmsg_only) {
> @@ -9715,7 +9717,7 @@ write_cmd_list(FILE *f, const char *branch_name,
>  	struct got_object_qid *qid;
>  
>  	qid = STAILQ_FIRST(commits);
> -	err = got_object_id_str(&id_str, qid->id);
> +	err = got_object_id_str(&id_str, &qid->id);
>  	if (err)
>  		return err;
>  
> @@ -10036,11 +10038,11 @@ histedit_check_script(struct got_histedit_list *histed
>  
>  	STAILQ_FOREACH(qid, commits, entry) {
>  		TAILQ_FOREACH(hle, histedit_cmds, entry) {
> -			if (got_object_id_cmp(qid->id, hle->commit_id) == 0)
> +			if (got_object_id_cmp(&qid->id, hle->commit_id) == 0)
>  				break;
>  		}
>  		if (hle == NULL) {
> -			err = got_object_id_str(&id_str, qid->id);
> +			err = got_object_id_str(&id_str, &qid->id);
>  			if (err)
>  				return err;
>  			snprintf(msg, sizeof(msg),
> @@ -10709,7 +10711,7 @@ cmd_histedit(int argc, char *argv[])
>  			error = got_error(GOT_ERR_EMPTY_HISTEDIT);
>  			goto done;
>  		}
> -		error = collect_commits(&commits, head_commit_id, pid->id,
> +		error = collect_commits(&commits, head_commit_id, &pid->id,
>  		    base_commit_id, got_worktree_get_path_prefix(worktree),
>  		    GOT_ERR_HISTEDIT_PATH, repo);
>  		got_object_commit_close(commit);
> @@ -10750,7 +10752,7 @@ cmd_histedit(int argc, char *argv[])
>  			error = got_error(GOT_ERR_EMPTY_HISTEDIT);
>  			goto done;
>  		}
> -		error = collect_commits(&commits, head_commit_id, pid->id,
> +		error = collect_commits(&commits, head_commit_id, &pid->id,
>  		    got_worktree_get_base_commit_id(worktree),
>  		    got_worktree_get_path_prefix(worktree),
>  		    GOT_ERR_HISTEDIT_PATH, repo);
> @@ -10880,7 +10882,7 @@ cmd_histedit(int argc, char *argv[])
>  		pid = STAILQ_FIRST(parent_ids);
>  
>  		error = got_worktree_histedit_merge_files(&merged_paths,
> -		    worktree, fileindex, pid->id, hle->commit_id, repo,
> +		    worktree, fileindex, &pid->id, hle->commit_id, repo,
>  		    update_progress, &upa, check_cancelled, NULL);
>  		if (error)
>  			goto done;
> @@ -11750,7 +11752,7 @@ cat_commit(struct got_object_id *id, struct got_reposi
>  	    got_object_commit_get_nparents(commit));
>  	STAILQ_FOREACH(pid, parent_ids, entry) {
>  		char *pid_str;
> -		err = got_object_id_str(&pid_str, pid->id);
> +		err = got_object_id_str(&pid_str, &pid->id);
>  		if (err)
>  			goto done;
>  		fprintf(outfile, "%s%s\n", GOT_COMMIT_LABEL_PARENT, pid_str);
> blob - d1549fbf070db1995686457305612b6d2905a8b6
> blob + 3d8c00182e1858eae16c7d8308e2673b82997bad
> --- gotweb/gotweb.c
> +++ gotweb/gotweb.c
> @@ -24,6 +24,7 @@
>  #include <err.h>
>  #include <errno.h>
>  #include <regex.h>
> +#include <sha1.h>
>  #include <stdarg.h>
>  #include <stdint.h>
>  #include <stdio.h>
> @@ -3703,7 +3704,7 @@ gw_get_commit(struct gw_trans *gw_trans, struct gw_hea
>  		parent_id = STAILQ_FIRST(
>  		    got_object_commit_get_parent_ids(commit));
>  		if (parent_id != NULL) {
> -			id2 = got_object_id_dup(parent_id->id);
> +			id2 = got_object_id_dup(&parent_id->id);
>  			free (parent_id);
>  			error = got_object_id_str(&header->parent_id, id2);
>  			if (error)
> blob - 9d6f362ad6242f2fdff94792bc113ede1710807d
> blob + 0a162cbd301d5069ad3612113a16489ad4b7fd87
> --- include/got_object.h
> +++ include/got_object.h
> @@ -14,7 +14,9 @@
>   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>   */
>  
> -struct got_object_id;
> +struct got_object_id {
> +	u_int8_t sha1[SHA1_DIGEST_LENGTH];
> +};
>  
>  struct got_blob_object;
>  struct got_tree_object;
> @@ -24,7 +26,7 @@ struct got_commit_object;
>  
>  struct got_object_qid {
>  	STAILQ_ENTRY(got_object_qid) entry;
> -	struct got_object_id *id;
> +	struct got_object_id id;
>  	void *data; /* managed by API user */
>  };
>  
> blob - 19208f0e07660099d682daaab42dfacd9c2c5724
> blob + 5e6b96770dafdf895ef7140b2fc28b3d2c8e370b
> --- lib/blame.c
> +++ lib/blame.c
> @@ -215,7 +215,7 @@ blame_commit(struct got_blame *blame, struct got_objec
>  		return NULL;
>  	}
>  
> -	err = got_object_open_as_commit(&pcommit, repo, pid->id);
> +	err = got_object_open_as_commit(&pcommit, repo, &pid->id);
>  	if (err)
>  		goto done;
>  
> blob - 644fd4c564841be0c8142f649c9e68f26133deb7
> blob + 01dc8264285fe44769741f2870eda27d3d1e6b16
> --- lib/commit_graph.c
> +++ lib/commit_graph.c
> @@ -126,7 +126,7 @@ detect_changed_path(int *changed, struct got_commit_ob
>  	if (err)
>  		return err;
>  
> -	err = got_object_open_as_commit(&pcommit, repo, pid->id);
> +	err = got_object_open_as_commit(&pcommit, repo, &pid->id);
>  	if (err)
>  		goto done;
>  
> @@ -215,9 +215,9 @@ packed_first_parent_traversal(int *ncommits_traversed,
>  	STAILQ_FOREACH(qid, &traversed_commits, entry) {
>  		struct got_commit_graph_node *node;
>  
> -		if (got_object_idset_contains(graph->open_branches, qid->id))
> +		if (got_object_idset_contains(graph->open_branches, &qid->id))
>  			continue;
> -		if (got_object_idset_contains(graph->node_ids, qid->id))
> +		if (got_object_idset_contains(graph->node_ids, &qid->id))
>  			continue;
>  
>  		(*ncommits_traversed)++;
> @@ -225,11 +225,11 @@ packed_first_parent_traversal(int *ncommits_traversed,
>  		/* ... except the last commit is the new branch tip. */
>  		if (STAILQ_NEXT(qid, entry) == NULL) {
>  			err = got_object_idset_add(graph->open_branches,
> -			    qid->id, NULL);
> +			    &qid->id, NULL);
>  			break;
>  		}
>  
> -		err = add_node(&node, graph, qid->id, repo);
> +		err = add_node(&node, graph, &qid->id, repo);
>  		if (err)
>  			break;
>  	}
> @@ -263,7 +263,7 @@ advance_branch(struct got_commit_graph *graph, struct 
>  	if (graph->flags & GOT_COMMIT_GRAPH_FIRST_PARENT_TRAVERSAL) {
>  		qid = STAILQ_FIRST(&commit->parent_ids);
>  		if (qid == NULL ||
> -		    got_object_idset_contains(graph->open_branches, qid->id))
> +		    got_object_idset_contains(graph->open_branches, &qid->id))
>  			return NULL;
>  		/*
>  		 * The root directory always changes by definition, and when
> @@ -277,12 +277,12 @@ advance_branch(struct got_commit_graph *graph, struct 
>  		    (commit->flags & GOT_COMMIT_FLAG_PACKED)) {
>  			int ncommits = 0;
>  			err = packed_first_parent_traversal(&ncommits,
> -			    graph, qid->id, repo);
> +			    graph, &qid->id, repo);
>  			if (err || ncommits > 0)
>  				return err;
>  		}
>  		return got_object_idset_add(graph->open_branches,
> -		    qid->id, NULL);
> +		    &qid->id, NULL);
>  	}
>  
>  	/*
> @@ -303,11 +303,11 @@ advance_branch(struct got_commit_graph *graph, struct 
>  			struct got_commit_object *pcommit = NULL;
>  
>  			if (got_object_idset_contains(graph->open_branches,
> -			    qid->id))
> +			    &qid->id))
>  				continue;
>  
>  			err = got_object_open_as_commit(&pcommit, repo,
> -			    qid->id);
> +			    &qid->id);
>  			if (err) {
>  				free(merged_id);
>  				free(prev_id);
> @@ -341,7 +341,7 @@ advance_branch(struct got_commit_graph *graph, struct 
>  			 */
>  			if (got_object_id_cmp(merged_id, id) == 0) {
>  				err = got_object_idset_add(graph->open_branches,
> -				    qid->id, NULL);
> +				    &qid->id, NULL);
>  				free(merged_id);
>  				free(id);
>  				return err;
> @@ -362,22 +362,23 @@ advance_branch(struct got_commit_graph *graph, struct 
>  			if (qid == NULL)
>  				return NULL;
>  			if (got_object_idset_contains(graph->open_branches,
> -			    qid->id))
> +			    &qid->id))
>  				return NULL;
>  			if (got_object_idset_contains(graph->node_ids,
> -			    qid->id))
> +			    &qid->id))
>  				return NULL; /* parent already traversed */
>  			return got_object_idset_add(graph->open_branches,
> -			    qid->id, NULL);
> +			    &qid->id, NULL);
>  		}
>  	}
>  
>  	STAILQ_FOREACH(qid, &commit->parent_ids, entry) {
> -		if (got_object_idset_contains(graph->open_branches, qid->id))
> +		if (got_object_idset_contains(graph->open_branches, &qid->id))
>  			continue;
> -		if (got_object_idset_contains(graph->node_ids, qid->id))
> +		if (got_object_idset_contains(graph->node_ids, &qid->id))
>  			continue; /* parent already traversed */
> -		err = got_object_idset_add(graph->open_branches, qid->id, NULL);
> +		err = got_object_idset_add(graph->open_branches, &qid->id,
> +		    NULL);
>  		if (err)
>  			return err;
>  	}
> blob - aa2b7c68c35e277c2b4e8eff9fad03155fada78e
> blob + f28b50b2a51b0e0f388b22fcf87b63f52408a2e1
> --- lib/diff3.c
> +++ lib/diff3.c
> @@ -69,6 +69,7 @@
>  
>  #include <ctype.h>
>  #include <limits.h>
> +#include <sha1.h>
>  #include <stdio.h>
>  #include <stdarg.h>
>  #include <stdlib.h>
> blob - 7bfd862b598c07fd5e72ed48db149670ae49ffbb
> blob + 8121609c04313ea17c7ca2a5e48e324aed510006
> --- lib/diffreg.c
> +++ lib/diffreg.c
> @@ -20,6 +20,7 @@
>  #include <sys/queue.h>
>  
>  #include <errno.h>
> +#include <sha1.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> blob - 7a4aaaaec7e976d89c7989136f80818dd4091850
> blob + 6af8d574c7b345c52d3e0c19759bf3ae6bd62b20
> --- lib/got_lib_object.h
> +++ lib/got_lib_object.h
> @@ -14,10 +14,6 @@
>   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>   */
>  
> -struct got_object_id {
> -	u_int8_t sha1[SHA1_DIGEST_LENGTH];
> -};
> -
>  struct got_object {
>  	int type;
>  
> blob - f6a83e12b20b326cac3cf14c4fe93d71d56a2775
> blob + b87e6eecb828ef98889452c3dd9b205e5eaf3c33
> --- lib/object.c
> +++ lib/object.c
> @@ -930,20 +930,11 @@ got_object_commit_open(struct got_commit_object **comm
>  const struct got_error *
>  got_object_qid_alloc(struct got_object_qid **qid, struct got_object_id *id)
>  {
> -	const struct got_error *err = NULL;
> -
>  	*qid = calloc(1, sizeof(**qid));
>  	if (*qid == NULL)
>  		return got_error_from_errno("calloc");
>  
> -	(*qid)->id = got_object_id_dup(id);
> -	if ((*qid)->id == NULL) {
> -		err = got_error_from_errno("got_object_id_dup");
> -		got_object_qid_free(*qid);
> -		*qid = NULL;
> -		return err;
> -	}
> -
> +	memcpy(&(*qid)->id, id, sizeof((*qid)->id));
>  	return NULL;
>  }
>  
> @@ -960,7 +951,7 @@ got_object_id_queue_copy(const struct got_object_id_qu
>  		 * Deep-copy the object ID only. Let the caller deal
>  		 * with setting up the new->data pointer if needed.
>  		 */
> -		err = got_object_qid_alloc(&new, qid->id); 
> +		err = got_object_qid_alloc(&new, &qid->id); 
>  		if (err) {
>  			got_object_id_queue_free(dest);
>  			return err;
> blob - c684ffd1f75dff183b69b5dc0492040c65d8338b
> blob + 644fb1ad06904cfc5d4c57cd3db0c88cd97119ea
> --- lib/object_cache.c
> +++ lib/object_cache.c
> @@ -122,7 +122,7 @@ get_size_commit(struct got_commit_object *commit)
>  	size += strlen(commit->logmsg);
>  
>  	STAILQ_FOREACH(qid, &commit->parent_ids, entry)
> -		size += sizeof(*qid) + sizeof(*qid->id);
> +		size += sizeof(*qid) + sizeof(qid->id);
>  
>  	return size;
>  }
> blob - 3102c04085d4e8c0157a4d3fd9c725a4b76f8c88
> blob + a3ad6737a9675482940881b8c700cfd61cb68e29
> --- lib/object_create.c
> +++ lib/object_create.c
> @@ -507,7 +507,7 @@ got_object_commit_create(struct got_object_id **id,
>  		STAILQ_FOREACH(qid, parent_ids, entry) {
>  			char *parent_str = NULL;
>  
> -			err = got_object_id_str(&id_str, qid->id);
> +			err = got_object_id_str(&id_str, &qid->id);
>  			if (err)
>  				goto done;
>  			if (asprintf(&parent_str, "%s%s\n",
> blob - e27d30ce0887da3caae5375fdd367eb524ddac3e
> blob + a6f27898584ea73a11f529273e3da9e6f5c10cf6
> --- lib/object_idset.c
> +++ lib/object_idset.c
> @@ -123,7 +123,7 @@ idset_resize(struct got_object_idset *set, size_t nbuc
>  			uint64_t idx;
>  			qid = STAILQ_FIRST(&set->ids[i]);
>  			STAILQ_REMOVE(&set->ids[i], qid, got_object_qid, entry);
> -			idx = idset_hash(set, qid->id) % nbuckets;
> +			idx = idset_hash(set, &qid->id) % nbuckets;
>  			STAILQ_INSERT_HEAD(&ids[idx], qid, entry);
>  		}
>  	}
> @@ -170,7 +170,7 @@ got_object_idset_add(struct got_object_idset *set, str
>  	err = got_object_qid_alloc_partial(&qid);
>  	if (err)
>  		return err;
> -	memcpy(qid->id, id, sizeof(*qid->id));
> +	memcpy(&qid->id, id, sizeof(qid->id));
>  	qid->data = data;
>  
>  	idx = idset_hash(set, id) % set->nbuckets;
> @@ -192,7 +192,7 @@ find_element(struct got_object_idset *set, struct got_
>  	struct got_object_qid *qid;
>  
>  	STAILQ_FOREACH(qid, head, entry) {
> -		if (got_object_id_cmp(qid->id, id) == 0)
> +		if (got_object_id_cmp(&qid->id, id) == 0)
>  			return qid;
>  	}
>  
> @@ -232,7 +232,7 @@ got_object_idset_remove(void **data, struct got_object
>  		idx = idset_hash(set, id) % set->nbuckets;
>  		head = &set->ids[idx];
>  		STAILQ_FOREACH(qid, head, entry) {
> -			if (got_object_id_cmp(qid->id, id) == 0)
> +			if (got_object_id_cmp(&qid->id, id) == 0)
>  				break;
>  		}
>  		if (qid == NULL)
> @@ -270,7 +270,7 @@ got_object_idset_for_each(struct got_object_idset *set
>  	for (i = 0; i < set->nbuckets; i++) {
>  		head = &set->ids[i];
>  		STAILQ_FOREACH_SAFE(qid, head, entry, tmp) {
> -			err = (*cb)(qid->id, qid->data, arg);
> +			err = (*cb)(&qid->id, qid->data, arg);
>  			if (err)
>  				goto done;
>  		}
> blob - 3b225ca9384c228bc4e2ed56af6e1fbb67ff45d7
> blob + 511053239b0a892dbb0de04e93ccd9ed8e69e7b8
> --- lib/object_parse.c
> +++ lib/object_parse.c
> @@ -78,21 +78,11 @@ got_object_id_cmp(const struct got_object_id *id1,
>  const struct got_error *
>  got_object_qid_alloc_partial(struct got_object_qid **qid)
>  {
> -	const struct got_error *err = NULL;
> -
>  	*qid = malloc(sizeof(**qid));
>  	if (*qid == NULL)
>  		return got_error_from_errno("malloc");
>  
> -	(*qid)->id = malloc(sizeof(*((*qid)->id)));
> -	if ((*qid)->id == NULL) {
> -		err = got_error_from_errno("malloc");
> -		got_object_qid_free(*qid);
> -		*qid = NULL;
> -		return err;
> -	}
>  	(*qid)->data = NULL;
> -
>  	return NULL;
>  }
>  
> @@ -164,7 +154,6 @@ got_object_raw_close(struct got_raw_object *obj)
>  void
>  got_object_qid_free(struct got_object_qid *qid)
>  {
> -	free(qid->id);
>  	free(qid);
>  }
>  
> @@ -314,7 +303,7 @@ got_object_commit_add_parent(struct got_commit_object 
>  	if (err)
>  		return err;
>  
> -	if (!got_parse_sha1_digest(qid->id->sha1, id_str)) {
> +	if (!got_parse_sha1_digest(qid->id.sha1, id_str)) {
>  		err = got_error(GOT_ERR_BAD_OBJ_DATA);
>  		got_object_qid_free(qid);
>  		return err;
> blob - 4aa9467c86b6e0a5f571584951e24abda6d8f5f5
> blob + d875046e25b7f0b4172baa4dbd064445a73f18c4
> --- lib/pack.c
> +++ lib/pack.c
> @@ -701,7 +701,7 @@ got_packidx_match_id_str_prefix(struct got_object_id_q
>  		err = got_object_qid_alloc_partial(&qid);
>  		if (err)
>  			break;
> -		memcpy(qid->id->sha1, oid->sha1, SHA1_DIGEST_LENGTH);
> +		memcpy(qid->id.sha1, oid->sha1, SHA1_DIGEST_LENGTH);
>  		STAILQ_INSERT_TAIL(matched_ids, qid, entry);
>  
>  		oid = &packidx->hdr.sorted_ids[++i];
> blob - 81518d57e3cdb40c14904ec873d086182fea8b4c
> blob + 14aa4357200fc7ebcefeb1306ac93ca9c8d7fb54
> --- lib/pack_create.c
> +++ lib/pack_create.c
> @@ -985,12 +985,12 @@ load_tree(int want_meta, struct got_object_idset *idse
>  		qid = STAILQ_FIRST(&tree_ids);
>  		STAILQ_REMOVE_HEAD(&tree_ids, entry);
>  
> -		if (got_object_idset_contains(idset, qid->id)) {
> +		if (got_object_idset_contains(idset, &qid->id)) {
>  			got_object_qid_free(qid);
>  			continue;
>  		}
>  
> -		err = add_object(want_meta, idset, qid->id, dpath,
> +		err = add_object(want_meta, idset, &qid->id, dpath,
>  		    GOT_OBJ_TYPE_TREE, mtime, loose_obj_only, repo,
>  		    ncolored, nfound, ntrees, progress_cb, progress_arg, rl);
>  		if (err) {
> @@ -998,7 +998,7 @@ load_tree(int want_meta, struct got_object_idset *idse
>  			break;
>  		}
>  
> -		err = load_tree_entries(&tree_ids, want_meta, idset, qid->id,
> +		err = load_tree_entries(&tree_ids, want_meta, idset, &qid->id,
>  		    dpath, mtime, repo, loose_obj_only, ncolored, nfound,
>  		    ntrees, progress_cb, progress_arg, rl,
>  		    cancel_cb, cancel_arg);
> @@ -1230,47 +1230,48 @@ paint_commits(int *ncolored, struct got_object_id_queu
>  		if (color == COLOR_SKIP)
>  			nskip--;
>  
> -		if (got_object_idset_contains(skip, qid->id)) {
> +		if (got_object_idset_contains(skip, &qid->id)) {
>  			got_object_qid_free(qid);
>  			continue;
>  		}
>  
>  		switch (color) {
>  		case COLOR_KEEP:
> -			if (got_object_idset_contains(keep, qid->id)) {
> +			if (got_object_idset_contains(keep, &qid->id)) {
>  				got_object_qid_free(qid);
>  				continue;
>  			}
> -			if (got_object_idset_contains(drop, qid->id)) {
> +			if (got_object_idset_contains(drop, &qid->id)) {
>  				err = paint_commit(qid, COLOR_SKIP);
>  				if (err)
>  					goto done;
>  				nskip++;
>  			} else
>  				(*ncolored)++;
> -			err = got_object_idset_add(keep, qid->id, NULL);
> +			err = got_object_idset_add(keep, &qid->id, NULL);
>  			if (err)
>  				goto done;
>  			break;
>  		case COLOR_DROP:
> -			if (got_object_idset_contains(drop, qid->id)) {
> +			if (got_object_idset_contains(drop, &qid->id)) {
>  				got_object_qid_free(qid);
>  				continue;
>  			}
> -			if (got_object_idset_contains(keep, qid->id)) {
> +			if (got_object_idset_contains(keep, &qid->id)) {
>  				err = paint_commit(qid, COLOR_SKIP);
>  				if (err)
>  					goto done;
>  				nskip++;
>  			} else
>  				(*ncolored)++;
> -			err = got_object_idset_add(drop, qid->id, NULL);
> +			err = got_object_idset_add(drop, &qid->id, NULL);
>  			if (err)
>  				goto done;
>  			break;
>  		case COLOR_SKIP:
> -			if (!got_object_idset_contains(skip, qid->id)) {
> -				err = got_object_idset_add(skip, qid->id, NULL);
> +			if (!got_object_idset_contains(skip, &qid->id)) {
> +				err = got_object_idset_add(skip, &qid->id,
> +				    NULL);
>  				if (err)
>  					goto done;
>  			}
> @@ -1288,7 +1289,7 @@ paint_commits(int *ncolored, struct got_object_id_queu
>  			break;
>  
>  
> -		err = got_object_open_as_commit(&commit, repo, qid->id);
> +		err = got_object_open_as_commit(&commit, repo, &qid->id);
>  		if (err)
>  			break;
>  
> @@ -1297,7 +1298,7 @@ paint_commits(int *ncolored, struct got_object_id_queu
>  			struct got_object_qid *pid;
>  			color = *((int *)qid->data);
>  			STAILQ_FOREACH(pid, parents, entry) {
> -				err = queue_commit_id(ids, pid->id, color,
> +				err = queue_commit_id(ids, &pid->id, color,
>  				    repo);
>  				if (err)
>  					break;
> blob - 67b0e54997c29a12feaca8d74bc968633b1711b4
> blob + de7f8e9eb12711a08545a3396b393be498822f4c
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -1308,7 +1308,7 @@ got_privsep_send_commit(struct imsgbuf *ibuf, struct g
>  	memcpy(buf + len, commit->committer, committer_len);
>  	len += committer_len;
>  	STAILQ_FOREACH(qid, &commit->parent_ids, entry) {
> -		memcpy(buf + len, qid->id, SHA1_DIGEST_LENGTH);
> +		memcpy(buf + len, &qid->id, SHA1_DIGEST_LENGTH);
>  		len += SHA1_DIGEST_LENGTH;
>  	}
>  
> @@ -1444,8 +1444,8 @@ get_commit_from_imsg(struct got_commit_object **commit
>  		err = got_object_qid_alloc_partial(&qid);
>  		if (err)
>  			break;
> -		memcpy(qid->id, imsg->data + len +
> -		    i * SHA1_DIGEST_LENGTH, sizeof(*qid->id));
> +		memcpy(&qid->id, imsg->data + len +
> +		    i * SHA1_DIGEST_LENGTH, sizeof(qid->id));
>  		STAILQ_INSERT_TAIL(&(*commit)->parent_ids, qid, entry);
>  		(*commit)->nparents++;
>  	}
> @@ -2688,13 +2688,13 @@ got_privsep_recv_traversed_commits(struct got_commit_o
>  				err = got_object_qid_alloc_partial(&qid);
>  				if (err)
>  					break;
> -				memcpy(qid->id->sha1, sha1, SHA1_DIGEST_LENGTH);
> +				memcpy(qid->id.sha1, sha1, SHA1_DIGEST_LENGTH);
>  				STAILQ_INSERT_TAIL(commit_ids, qid, entry);
>  
>  				/* The last commit may contain a change. */
>  				if (i == icommits->ncommits - 1) {
>  					*changed_commit_id =
> -					    got_object_id_dup(qid->id);
> +					    got_object_id_dup(&qid->id);
>  					if (*changed_commit_id == NULL) {
>  						err = got_error_from_errno(
>  						    "got_object_id_dup");
> blob - 634a1c3ab99f87c92f95463d1a11f669d8950830
> blob + 7896e9194a183430cbeecf89b54758b1e9b1dc94
> --- lib/repository.c
> +++ lib/repository.c
> @@ -1480,20 +1480,21 @@ match_packed_object(struct got_object_id **unique_id,
>  			if (obj_type != GOT_OBJ_TYPE_ANY) {
>  				int matched_type;
>  				err = got_object_get_type(&matched_type, repo,
> -				    qid->id);
> +				    &qid->id);
>  				if (err)
>  					goto done;
>  				if (matched_type != obj_type)
>  					continue;
>  			}
>  			if (*unique_id == NULL) {
> -				*unique_id = got_object_id_dup(qid->id);
> +				*unique_id = got_object_id_dup(&qid->id);
>  				if (*unique_id == NULL) {
>  					err = got_error_from_errno("malloc");
>  					goto done;
>  				}
>  			} else {
> -				if (got_object_id_cmp(*unique_id, qid->id) == 0)
> +				if (got_object_id_cmp(*unique_id,
> +				    &qid->id) == 0)
>  					continue; /* packed multiple times */
>  				err = got_error(GOT_ERR_AMBIGUOUS_ID);
>  				goto done;
> blob - a2cd689e950dde612d9eb621415378868483d8bf
> blob + badde305405fbbe8f81fa4406548c06f1ff13375
> --- lib/repository_admin.c
> +++ lib/repository_admin.c
> @@ -869,24 +869,24 @@ load_tree(struct got_object_idset *loose_ids,
>  		qid = STAILQ_FIRST(&tree_ids);
>  		STAILQ_REMOVE_HEAD(&tree_ids, entry);
>  
> -		if (got_object_idset_contains(traversed_ids, qid->id)) {
> +		if (got_object_idset_contains(traversed_ids, &qid->id)) {
>  			got_object_qid_free(qid);
>  			continue;
>  		}
>  
> -		err = got_object_idset_add(traversed_ids, qid->id, NULL);
> +		err = got_object_idset_add(traversed_ids, &qid->id, NULL);
>  		if (err) {
>  			got_object_qid_free(qid);
>  			break;
>  		}
>  
>  		/* This tree is referenced. */
> -		err = preserve_loose_object(loose_ids, qid->id, repo, npacked);
> +		err = preserve_loose_object(loose_ids, &qid->id, repo, npacked);
>  		if (err)
>  			break;
>  
>  		err = load_tree_entries(&tree_ids, loose_ids, traversed_ids,
> -		    qid->id, dpath, repo, npacked, cancel_cb, cancel_arg);
> +		    &qid->id, dpath, repo, npacked, cancel_cb, cancel_arg);
>  		got_object_qid_free(qid);
>  		if (err)
>  			break;
> @@ -929,32 +929,33 @@ load_commit_or_tag(struct got_object_idset *loose_ids,
>  		qid = STAILQ_FIRST(&ids);
>  		STAILQ_REMOVE_HEAD(&ids, entry);
>  
> -		if (got_object_idset_contains(traversed_ids, qid->id)) {
> +		if (got_object_idset_contains(traversed_ids, &qid->id)) {
>  			got_object_qid_free(qid);
>  			qid = NULL;
>  			continue;
>  		}
>  
> -		err = got_object_idset_add(traversed_ids, qid->id, NULL);
> +		err = got_object_idset_add(traversed_ids, &qid->id, NULL);
>  		if (err)
>  			break;
>  
>  		/* This commit or tag is referenced. */
> -		err = preserve_loose_object(loose_ids, qid->id, repo, npacked);
> +		err = preserve_loose_object(loose_ids, &qid->id, repo, npacked);
>  		if (err)
>  			break;
>  
> -		err = got_object_get_type(&obj_type, repo, qid->id);
> +		err = got_object_get_type(&obj_type, repo, &qid->id);
>  		if (err)
>  			break;
>  		switch (obj_type) {
>  		case GOT_OBJ_TYPE_COMMIT:
> -			err = got_object_open_as_commit(&commit, repo, qid->id);
> +			err = got_object_open_as_commit(&commit, repo,
> +			    &qid->id);
>  			if (err)
>  				goto done;
>  			break;
>  		case GOT_OBJ_TYPE_TAG:
> -			err = got_object_open_as_tag(&tag, repo, qid->id);
> +			err = got_object_open_as_tag(&tag, repo, &qid->id);
>  			if (err)
>  				goto done;
>  			break;
> @@ -987,7 +988,7 @@ load_commit_or_tag(struct got_object_idset *loose_ids,
>  				 * and the object it points to on disk.
>  				 */
>  				err = got_object_idset_remove(NULL, loose_ids,
> -				    qid->id);
> +				    &qid->id);
>  				if (err && err->code != GOT_ERR_NO_OBJ)
>  					goto done;
>  				err = got_object_idset_remove(NULL, loose_ids,
> blob - 965700c2ad7edf85000a045f92b3727fc9ebc460
> blob + 673cbeb89c705904097774881d38ef9d51751939
> --- lib/worktree_open.c
> +++ lib/worktree_open.c
> @@ -20,6 +20,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <limits.h>
> +#include <sha1.h>
>  #include <stddef.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> blob - a14d051c87c9e8f5b75dd5f4bcf67486dfde5f4c
> blob + b014128a336645e26ebddf6b06cecbdc652fc1b3
> --- libexec/got-read-pack/got-read-pack.c
> +++ libexec/got-read-pack/got-read-pack.c
> @@ -683,11 +683,11 @@ commit_traversal_request(struct imsg *imsg, struct ims
>  		if (pid == NULL)
>  			break;
>  
> -		idx = got_packidx_get_object_idx(packidx, pid->id);
> +		idx = got_packidx_get_object_idx(packidx, &pid->id);
>  		if (idx == -1)
>  			break;
>  
> -		err = open_commit(&pcommit, pack, packidx, idx, pid->id,
> +		err = open_commit(&pcommit, pack, packidx, idx, &pid->id,
>  		    objcache);
>  		if (err) {
>  			if (err->code != GOT_ERR_NO_OBJ)
> @@ -745,7 +745,7 @@ commit_traversal_request(struct imsg *imsg, struct ims
>  		}
>  
>  		if (!changed) {
> -			memcpy(id.sha1, pid->id->sha1, SHA1_DIGEST_LENGTH);
> +			memcpy(id.sha1, pid->id.sha1, SHA1_DIGEST_LENGTH);
>  			got_object_commit_close(commit);
>  			commit = pcommit;
>  			pcommit = NULL;
> blob - 1235f3d15f5c154123999f3791ee35c3119cd238
> blob + a4221a9f756b84c1412ef1f4983cccd6aa233c2d
> --- tog/tog.c
> +++ tog/tog.c
> @@ -24,6 +24,7 @@
>  #include <curses.h>
>  #include <panel.h>
>  #include <locale.h>
> +#include <sha1.h>
>  #include <signal.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
> @@ -1880,7 +1881,7 @@ open_diff_view_for_commit(struct tog_view **new_view, 
>  		return got_error_from_errno("view_open");
>  
>  	parent_id = STAILQ_FIRST(got_object_commit_get_parent_ids(commit));
> -	err = open_diff_view(diff_view, parent_id ? parent_id->id : NULL,
> +	err = open_diff_view(diff_view, parent_id ? &parent_id->id : NULL,
>  	    commit_id, NULL, NULL, 3, 0, 0, log_view, repo);
>  	if (err == NULL)
>  		*new_view = diff_view;
> @@ -3118,7 +3119,7 @@ get_changed_paths(struct got_pathlist_head *paths,
>  	if (qid != NULL) {
>  		struct got_commit_object *pcommit;
>  		err = got_object_open_as_commit(&pcommit, repo,
> -		    qid->id);
> +		    &qid->id);
>  		if (err)
>  			return err;
>  
> @@ -3261,7 +3262,7 @@ write_commit_info(off_t **line_offsets, size_t *nlines
>  		int pn = 1;
>  		parent_ids = got_object_commit_get_parent_ids(commit);
>  		STAILQ_FOREACH(qid, parent_ids, entry) {
> -			err = got_object_id_str(&id_str, qid->id);
> +			err = got_object_id_str(&id_str, &qid->id);
>  			if (err)
>  				goto done;
>  			n = fprintf(outfile, "parent %d: %s\n", pn++, id_str);
> @@ -3390,7 +3391,7 @@ create_diff(struct tog_diff_view_state *s)
>  		} else {
>  			parent_ids = got_object_commit_get_parent_ids(commit2);
>  			STAILQ_FOREACH(pid, parent_ids, entry) {
> -				if (got_object_id_cmp(s->id1, pid->id) == 0) {
> +				if (got_object_id_cmp(s->id1, &pid->id) == 0) {
>  					err = write_commit_info(
>  					    &s->line_offsets, &s->nlines,
>  					    s->id2, refs, s->repo, s->f);
> @@ -3699,7 +3700,7 @@ set_selected_commit(struct tog_diff_view_state *s,
>  	parent_ids = got_object_commit_get_parent_ids(selected_commit);
>  	free(s->id1);
>  	pid = STAILQ_FIRST(parent_ids);
> -	s->id1 = pid ? got_object_id_dup(pid->id) : NULL;
> +	s->id1 = pid ? got_object_id_dup(&pid->id) : NULL;
>  	got_object_commit_close(selected_commit);
>  	return NULL;
>  }
> @@ -4014,7 +4015,7 @@ draw_blame(struct tog_view *view)
>  	char *id_str;
>  	struct tog_color *tc;
>  
> -	err = got_object_id_str(&id_str, s->blamed_commit->id);
> +	err = got_object_id_str(&id_str, &s->blamed_commit->id);
>  	if (err)
>  		return err;
>  
> @@ -4337,7 +4338,7 @@ run_blame(struct tog_view *view)
>  	int obj_type;
>  
>  	err = got_object_open_as_commit(&commit, s->repo,
> -	    s->blamed_commit->id);
> +	    &s->blamed_commit->id);
>  	if (err)
>  		return err;
>  
> @@ -4388,7 +4389,7 @@ run_blame(struct tog_view *view)
>  	blame->cb_args.view = view;
>  	blame->cb_args.lines = blame->lines;
>  	blame->cb_args.nlines = blame->nlines;
> -	blame->cb_args.commit_id = got_object_id_dup(s->blamed_commit->id);
> +	blame->cb_args.commit_id = got_object_id_dup(&s->blamed_commit->id);
>  	if (blame->cb_args.commit_id == NULL) {
>  		err = got_error_from_errno("got_object_id_dup");
>  		goto done;
> @@ -4670,7 +4671,7 @@ input_blame_view(struct tog_view **new_view, struct to
>  			}
>  			/* Check if path history ends here. */
>  			err = got_object_open_as_commit(&pcommit,
> -			    s->repo, pid->id);
> +			    s->repo, &pid->id);
>  			if (err)
>  				break;
>  			err = got_object_id_by_path(&blob_id, s->repo,
> @@ -4691,11 +4692,11 @@ input_blame_view(struct tog_view **new_view, struct to
>  				break;
>  			}
>  			err = got_object_qid_alloc(&s->blamed_commit,
> -			    pid->id);
> +			    &pid->id);
>  			got_object_commit_close(commit);
>  		} else {
>  			if (got_object_id_cmp(id,
> -			    s->blamed_commit->id) == 0)
> +			    &s->blamed_commit->id) == 0)
>  				break;
>  			err = got_object_qid_alloc(&s->blamed_commit,
>  			    id);
> @@ -4717,7 +4718,7 @@ input_blame_view(struct tog_view **new_view, struct to
>  	case 'B': {
>  		struct got_object_qid *first;
>  		first = STAILQ_FIRST(&s->blamed_commits);
> -		if (!got_object_id_cmp(first->id, s->commit_id))
> +		if (!got_object_id_cmp(&first->id, s->commit_id))
>  			break;
>  		s->done = 1;
>  		thread_err = stop_blame(&s->blame);
> @@ -4755,7 +4756,7 @@ input_blame_view(struct tog_view **new_view, struct to
>  			err = got_error_from_errno("view_open");
>  			break;
>  		}
> -		err = open_diff_view(diff_view, pid ? pid->id : NULL,
> +		err = open_diff_view(diff_view, pid ? &pid->id : NULL,
>  		    id, NULL, NULL, 3, 0, 0, NULL, s->repo);
>  		got_object_commit_close(commit);
>  		if (err) {