Skip to content

perf(perf-map-agent): index the git root once instead of walking it per source lookup #17

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.

Sign up for GitHub

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

Open
not-matthias wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(perf-map-agent): index the git root once instead of walking it per source lookup #17

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
163 changes: 119 additions & 44 deletions jmh-fork/jmh-core/native-perf-map-agent/src/main/c/perf_map_agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
* perf_map_agent.c — JVMTI agent that writes /tmp/perf-<pid>.map
*
* Hooks CompiledMethodLoad events and writes perf map entries with
* absolute source file paths resolved via git root discovery.
* absolute source file paths resolved against the enclosing git repository.
*
* The git root is walked exactly once, on the first lookup, into a
* basename-keyed index; every later resolution is an in-memory bucket scan.
* CompiledMethodLoad is delivered on the JVM service thread while user code
* runs, so a filesystem walk per lookup would compete with the workload being
* measured.
*
* Usage:
* java -agentpath:libperf_map_agent.so[=file=<path>] ...
Expand Down Expand Up @@ -74,7 +80,6 @@ static const char *cache_insert(const char *key, const char *value) {
/* -------------------------------------------------------------------------- */

static char git_root[PATH_MAX];
static pthread_once_t git_root_once = PTHREAD_ONCE_INIT;

static void find_git_root(void) {
char cwd[PATH_MAX];
Expand Down Expand Up @@ -113,9 +118,22 @@ static void find_git_root(void) {
}

/* -------------------------------------------------------------------------- */
/* Recursive file search */
/* Source file index */
/* -------------------------------------------------------------------------- */

#define SOURCE_INDEX_BUCKETS \
4096 /* power of two, so mask instead of modulo \
*/

typedef struct index_entry {
char *path; /* absolute path of an indexed regular file */
struct index_entry *next;
} index_entry_t;

/* Chained buckets, keyed by the hash of the file's basename. */
static index_entry_t *source_index[SOURCE_INDEX_BUCKETS];
static pthread_once_t source_index_once = PTHREAD_ONCE_INIT;

static int should_skip_dir(const char *name) {
if (name[0] == '.') {
return 1;
Expand All @@ -127,15 +145,48 @@ static int should_skip_dir(const char *name) {
return 0;
}

/* FNV-1a over the basename, folded into a bucket number. */
static uint32_t hash_basename(const char *name) {
uint32_t h = 2166136261u;
for (; *name != '\0'; name++) {
h ^= (unsigned char)*name;
h *= 16777619u;
}
return h & (SOURCE_INDEX_BUCKETS - 1);
}

static const char *path_basename(const char *path) {
const char *slash = strrchr(path, '/');
return slash ? slash + 1 : path;
}

static void index_insert(const char *abs_path) {
index_entry_t *e = malloc(sizeof(*e));
if (!e) {
return;
}
e->path = strdup(abs_path);
if (!e->path) {
free(e);
return;
}

uint32_t bucket = hash_basename(path_basename(abs_path));
e->next = source_index[bucket];
source_index[bucket] = e;
Comment on lines +175 to +176

@greptile-apps greptile-apps Bot Sep 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Duplicate paths resolve differently

If a multi-module checkout has the same package-relative file in two source roots, such as com/example/Foo.java, prepending each entry reverses their traversal order. index_find therefore returns the last discovered match, while the previous recursive search returned the first. The perf map can consequently associate a compiled method with a different source file.

Suggested change
e->next = source_index[bucket];
source_index[bucket] = e;
e->next = NULL;
index_entry_t **tail = &source_index[bucket];
while (*tail) {
tail = &(*tail)->next;
}
*tail = e;
Prompt To Fix With AI 
This is a comment left during a code review.
Path: jmh-fork/jmh-core/native-perf-map-agent/src/main/c/perf_map_agent.c
Line: 175-176

Comment:
**Duplicate paths resolve differently**

If a multi-module checkout has the same package-relative file in two source roots, such as `com/example/Foo.java`, prepending each entry reverses their traversal order. `index_find` therefore returns the last discovered match, while the previous recursive search returned the first. The perf map can consequently associate a compiled method with a different source file.

```suggestion
  e->next = NULL;
  index_entry_t **tail = &source_index[bucket];
  while (*tail) {
    tail = &(*tail)->next;
  }
  *tail = e;
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

}

/*
* Search for a file whose path ends with `suffix` under `dir`.
* Returns 1 if found (result written to `out`), 0 otherwise.
* Index every regular file under `dir`, recursing into subdirectories that
* `should_skip_dir` accepts.
*
* `d_type` spares a stat() per entry; entry types the filesystem does not
* report, and symlinks (which are followed), fall back to stat().
*/
static int find_file_recursive(const char *dir, const char *suffix, char *out,
size_t out_size) {
static void index_dir(const char *dir) {
DIR *d = opendir(dir);
if (!d) {
return 0;
return;
}

struct dirent *ent;
Expand All @@ -147,34 +198,62 @@ static int find_file_recursive(const char *dir, const char *suffix, char *out,
char full_path[PATH_MAX];
snprintf(full_path, sizeof(full_path), "%s/%s", dir, ent->d_name);

struct stat st;
if (stat(full_path, &st) != 0) {
continue;
}

if (S_ISDIR(st.st_mode)) {
if (should_skip_dir(ent->d_name)) {
int is_dir = (ent->d_type == DT_DIR);
int is_reg = (ent->d_type == DT_REG);
if (!is_dir && !is_reg) {
struct stat st;
if (stat(full_path, &st) != 0) {
continue;
}
if (find_file_recursive(full_path, suffix, out, out_size)) {
closedir(d);
return 1;
}
} else if (S_ISREG(st.st_mode)) {
size_t full_len = strlen(full_path);
size_t suffix_len = strlen(suffix);
if (full_len >= suffix_len &&
strcmp(full_path + full_len - suffix_len, suffix) == 0) {
strncpy(out, full_path, out_size - 1);
out[out_size - 1] = '\0';
closedir(d);
return 1;
}
is_dir = S_ISDIR(st.st_mode);
is_reg = S_ISREG(st.st_mode);
}

if (is_dir && !should_skip_dir(ent->d_name)) {
index_dir(full_path);
} else if (is_reg) {
index_insert(full_path);
}
}

closedir(d);
return 0;
}

static void source_index_init(void) {
find_git_root();
index_dir(git_root);
}

static void index_free(void) {
for (size_t i = 0; i < SOURCE_INDEX_BUCKETS; i++) {
index_entry_t *e = source_index[i];
while (e) {
index_entry_t *next = e->next;
free(e->path);
free(e);
e = next;
}
source_index[i] = NULL;
}
}

/*
* Find an indexed file whose absolute path ends with "/<relative_path>".
* Returns NULL when no indexed file matches.
*/
static const char *index_find(const char *relative_path) {
char suffix[PATH_MAX];
snprintf(suffix, sizeof(suffix), "/%s", relative_path);
size_t suffix_len = strlen(suffix);

uint32_t bucket = hash_basename(path_basename(relative_path));
for (index_entry_t *e = source_index[bucket]; e; e = e->next) {
size_t len = strlen(e->path);
if (len >= suffix_len && strcmp(e->path + len - suffix_len, suffix) == 0) {
return e->path;
}
}
return NULL;
}

/* -------------------------------------------------------------------------- */
Expand All @@ -183,7 +262,11 @@ static int find_file_recursive(const char *dir, const char *suffix, char *out,

/*
* Resolve a class-relative path (e.g. "com/example/Foo.java") to an absolute
* path on disk by searching from the git root.
* path on disk.
*
* The first call walks the git root once and builds the basename index; later
* calls only scan one bucket, and the per-relative-path cache below keeps even
* that off the repeated-lookup path while handing out stable pointers.
*
* Returns a pointer that remains valid for the process lifetime.
* Empty string means "source file not found on disk".
Expand All @@ -197,20 +280,11 @@ static const char *resolve_source_file(const char *relative_path) {
return cached;
}

pthread_once(&git_root_once, find_git_root);

/* Build the suffix to search for: "/com/example/Foo.java" */
char suffix[PATH_MAX];
snprintf(suffix, sizeof(suffix), "/%s", relative_path);
pthread_once(&source_index_once, source_index_init);

char found[PATH_MAX];
const char *result = NULL;
if (find_file_recursive(git_root, suffix, found, sizeof(found))) {
result = cache_insert(relative_path, found);
} else {
/* Source not on disk (JDK class, dependency, etc.) */
result = cache_insert(relative_path, "");
}
/* Empty value: source not on disk (JDK class, dependency, etc.) */
const char *found = index_find(relative_path);
const char *result = cache_insert(relative_path, found ? found : "");

pthread_mutex_unlock(&cache_lock);
return result ? result : "";
Expand Down Expand Up @@ -578,5 +652,6 @@ JNIEXPORT void JNICALL Agent_OnUnload(JavaVM *jvm) {
e = next;
}
source_cache = NULL;
index_free();
pthread_mutex_unlock(&cache_lock);
}
Loading