Skip to content

trace2:gvfs:experiment Add experimental regions and data events to help diagnose checkout and reset perf problems #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions builtin/checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "submodule-config.h"
#include "submodule.h"
#include "advice.h"
#include "packfile.h"

static int checkout_optimize_new_branch;

Expand Down Expand Up @@ -921,8 +922,13 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
strbuf_release(&msg);
if (!opts->quiet &&
(new_branch_info->path || (!opts->force_detach && !strcmp(new_branch_info->name, "HEAD")))) {
unsigned long nr_unpack_entry_at_start;

trace2_region_enter("exp", "report_tracking", the_repository);
nr_unpack_entry_at_start = get_nr_unpack_entry();
report_tracking(new_branch_info);
trace2_data_intmax("exp", NULL, "report_tracking/nr_unpack_entries",
(intmax_t)(get_nr_unpack_entry() - nr_unpack_entry_at_start));
trace2_region_leave("exp", "report_tracking", the_repository);
}
}
Expand Down
15 changes: 13 additions & 2 deletions cache-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,20 +221,31 @@ static void discard_unused_subtrees(struct cache_tree *it)
}
}

int cache_tree_fully_valid(struct cache_tree *it)
static int cache_tree_fully_valid_1(struct cache_tree *it)
{
int i;
if (!it)
return 0;
if (it->entry_count < 0 || !has_object_file(&it->oid))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
if (!cache_tree_fully_valid(it->down[i]->cache_tree))
if (!cache_tree_fully_valid_1(it->down[i]->cache_tree))
return 0;
}
return 1;
}

int cache_tree_fully_valid(struct cache_tree *it)
{
int result;

trace2_region_enter("cache_tree", "fully_valid", NULL);
result = cache_tree_fully_valid_1(it);
trace2_region_leave("cache_tree", "fully_valid", NULL);

return result;
}

static int update_one(struct cache_tree *it,
struct cache_entry **cache,
int entries,
Expand Down
9 changes: 9 additions & 0 deletions packfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -1646,6 +1646,13 @@ static void *read_object(struct repository *r,
return content;
}

static unsigned long g_nr_unpack_entry;

unsigned long get_nr_unpack_entry(void)
{
return g_nr_unpack_entry;
}

void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
enum object_type *final_type, unsigned long *final_size)
{
Expand All @@ -1659,6 +1666,8 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC;
int base_from_cache = 0;

g_nr_unpack_entry++;

write_pack_access_log(p, obj_offset);

/* PHASE 1: drill down to the innermost base object */
Expand Down
5 changes: 5 additions & 0 deletions packfile.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,9 @@ int is_promisor_object(const struct object_id *oid);
int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
size_t idx_size, struct packed_git *p);

/*
* Return the number of objects fetched from a packfile.
*/
unsigned long get_nr_unpack_entry(void);

#endif
8 changes: 8 additions & 0 deletions unpack-trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "fetch-object.h"
#include "gvfs.h"
#include "virtualfilesystem.h"
#include "packfile.h"

/*
* Error messages expected by scripts out of plumbing commands such as
Expand Down Expand Up @@ -1474,10 +1475,14 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
int i, ret;
static struct cache_entry *dfc;
struct exclude_list el;
unsigned long nr_unpack_entry_at_start;

if (len > MAX_UNPACK_TREES)
die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);

trace2_region_enter("exp", "unpack_trees", NULL);
nr_unpack_entry_at_start = get_nr_unpack_entry();

trace_performance_enter();
memset(&el, 0, sizeof(el));
if (!core_apply_sparse_checkout || !o->update)
Expand Down Expand Up @@ -1658,6 +1663,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
done:
trace_performance_leave("unpack_trees");
clear_exclude_list(&el);
trace2_data_intmax("exp", NULL, "unpack_trees/nr_unpack_entries",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that sometimes, the experiments are prefixed with "exp", but sometimes not. Did you mean to prefix all of them in that manner, or was it intentional to do that only with some, but not others?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i've been inconsistent. i was using "exp" for things i consider really temporary while we gather some data for a release or 2. most of the other ones that have real categories, but have "experiment" in the commit message are things that i could keep and maybe upstream or could just drop. i'm trying to not flood the telemetry stream with things that may not be useful long term, but at the same time gather enough data for our immediate analysis needs.

(intmax_t)(get_nr_unpack_entry() - nr_unpack_entry_at_start));
trace2_region_leave("exp", "unpack_trees", NULL);
return ret;

return_failed:
Expand Down
4 changes: 4 additions & 0 deletions virtualfilesystem.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ void apply_virtualfilesystem(struct index_state *istate)
if (!git_config_get_virtualfilesystem())
return;

trace2_region_enter("vfs", "apply", the_repository);

if (!virtual_filesystem_data.len)
get_virtual_filesystem_data(&virtual_filesystem_data);

Expand Down Expand Up @@ -329,6 +331,8 @@ void apply_virtualfilesystem(struct index_state *istate)
trace2_data_intmax("vfs", the_repository, "apply/nr_bulk_skip", nr_bulk_skip);
trace2_data_intmax("vfs", the_repository, "apply/nr_explicit_skip", nr_explicit_skip);
}

trace2_region_leave("vfs", "apply", the_repository);
}

/*
Expand Down