Remove GLib dependency within bin/ #184

mack-w · 2025-05-15T17:45:15Z

This commit is the first in a series of commits to remove gLib. This commit moves an unused thread pool library within traceAnalyzer to the utils directory, which may find its use elsewhere.

This reverts commit 1155efd.

This commit is the first in series (ignoring reverted) to remove gLib. This commit introduces [hashmap.h](https://github.com/sheredom/hashmap.h) in an effort to remove GHashTable dependencies from the project. Future commits shall introduce similar header-only libs as well.

1a1a11a · 2025-05-16T14:25:58Z

just FYI, it might be better to start with a small PR :)

mack-w · 2025-05-16T16:09:11Z

Thanks for the suggestion. So let's begin with something like "remove GLib dependency from main executable" first? Since gLib usage within the algo part is more extensive and complicated, maybe I shall get rid of GLib in bin/ first (and I'm close to that.)

BTW the quality gate check is complaining about duplicated code on hashmap.h... it's saying like "for i=0" is a duplicate. I guess I'm gonna ignore that unless there're other problems with cq.

Move from GThreadPool to a homebrewed threadpool adapted from https://nachtimwald.com/2019/04/12/thread-pool-in-c Note that this code **only** runs with UBSan enabled. Not for production use (yet).

1a1a11a · 2025-05-17T19:33:21Z

Sounds good!

Finalize move from GThreadPool to a homebrewed threadpool adapted from https://nachtimwald.com/2019/04/12/thread-pool-in-c Note, that simulation results differ from original. Reason for this is under investigation.

mack-w · 2025-05-19T10:46:13Z

@1a1a11a Hi! I would say that this PR is near finished.

In the meantime, I'm observing an at-most 1% difference in simulation results for some algorithms (e.g. FIFO) with mrcProfiler. I also see assertion error hit_size_vec[9] in testMrcProfiler::test_minisim_profiler(). I'm still investigating the reason for this.

Right now sanitizers are not throwing any useful error, and vvverbose logs look good as well. Since testMrcProfiler is single-threaded, there shall be no data races. I will try to git bisect what's going wrong here. In the meantime if you have any hints or you want to see the logs please reply.

PS: that strncpy security hotspot emerged out of the blue and I ASSERT_IT_IS_SAFE

1a1a11a · 2025-05-19T12:30:03Z

loop in @JasonGuo98

Copilot

Pull Request Overview

This PR is focused on removing GLib dependencies in favor of using native pthreads and custom data structures. Key changes include replacing GLib’s GMutex, GThreadPool, and GHashTable with pthread-based synchronization primitives and custom thread pool/hashmap implementations, as well as updating include paths and launch configurations.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
libCacheSim/traceReader/reader.c	Replace non-thread safe boolean flag with a volatile qualifier
libCacheSim/profiler/threadpool.c	Introduce custom threadpool implementation adapted from external source
libCacheSim/profiler/simulator.c	Replace GThreadPool and GMutex with pthread equivalents and update index conversion
libCacheSim/profiler/profilerLRU.c	Switch from g_hash_table to custom hashmap functions
libCacheSim/profiler/dist.c	Replace GLib hash table calls with custom hashmap functions
libCacheSim/include/libCacheSim/threadpool.h	New header for threadpool API
libCacheSim/include/libCacheSim/hashmap_defs.in	New header for hashmap definitions
.vscode/launch.json	Updated executable path and arguments

libCacheSim/traceReader/reader.c

Copilot · 2025-05-19T12:31:30Z

libCacheSim/profiler/simulator.c

  sim_mt_params_t *params = (sim_mt_params_t *)user_data;
-  int idx = GPOINTER_TO_UINT(data) - 1;
+  // int idx = GPOINTER_TO_UINT(data) - 1;
+  int idx = ((unsigned int)(unsigned long)(data)) - 1;


The conversion from a pointer to an unsigned integer for index calculation may be error-prone on platforms where pointer sizes differ. Consider using a safer, more portable approach for this conversion to prevent potential issues.

Either we pass pointer to int, or we use size_t in place of unsigned long.
If we don't care about portability to 32-bit platforms or non amd64/aarch64, we just ignore this.
@1a1a11a What do you think?

mack-w · 2025-05-19T16:07:38Z

loop in @JasonGuo98

Did a quick test, it seems that the thread pool library that I'm quoting is problematic. I switched to a battle-tested thread pool library, and (from preliminary test results) the problem is gone :)

Gotta do some git rebase afterwards...

For the deeper side of the problem, I suspect it's still the synchronisation problem with the reader that I'm not coping well with., since TSan reports a data race issue there. It is a benign problem at first glance, but who knows. Pushing a new commit to fix this soon.

Upd: seems more like a problem with simulation steps, rather than reader problem. Introduced with the latest commit. Overlooked the problem since TSan keeps reporting the other way.

mack-w · 2025-05-20T11:08:07Z

Add these two lines to respective functions in simulator.c:

INFO("actual progress %d\n", progress);
INFO("actual num_of_caches %d\n", num_of_caches);

Before commit af3ff57:

[INFO]  simulator.c:307  (tid=126433726604224): actual progress 10
[INFO]  simulator.c:308  (tid=126433726604224): actual num_of_caches 10
ok 3 /libCacheSim/test_minisim_profiler_with_fixed_sample_rate

After commit af3ff57:

[INFO]  simulator.c:350  (tid=130706295354304): actual progress 9
[INFO]  simulator.c:351  (tid=130706295354304): actual num_of_caches 10
ERROR:test/test_mrcProfiler.cpp:188:void test_minisim_profiler_with_fixed_sample_rate(gconstpointer): assertion failed
not ok 3 /libCacheSim/test_minisim_profiler_with_fixed_sample_rate

The logic of the original code says:

while (progress < num_of_caches - 1) {
    print_progress((double)progress / (double)(num_of_caches - 1) * 100);
}

It seems that progress shall be num_of_caches minus 1, but with the original code (upstream HEAD) it is not the case. Is it designed like this on purpose? Or is it just a data race problem, as TSan reported?

Update: it is designed for purpose, my bad. Pushing a fix.

Anyways, marking this PR as ready for review.

The previous commit af3ff57 introduced pthread_cond_t to synchronize progress. However, in some circumstances, the simulate_* functions may exit prematurely due to incorrect conditional wait. This commit fix the abovementioned problem.

1a1a11a · 2025-05-22T00:50:22Z

libCacheSim/bin/cli_reader_utils.c

Can you add some documentation on how to use the data structures in the repo?

Actually hashmap.h is well documented. I shall then add code documentation and brief README for threadpool.h

1a1a11a · 2025-05-22T00:51:24Z

libCacheSim/bin/cli_reader_utils.c

@@ -267,11 +274,12 @@ void cal_working_set_size(reader_t *reader, int64_t *wss_obj,
      continue;
    }

-    if (g_hash_table_contains(obj_table, (gconstpointer)req->obj_id)) {
+    if (hashmap_get(&new_obj_table, (const void *)(req->obj_id),


Can we just use obj_id as the key?

Yes, it is done as so. There is a custom hasher for obj_id (that I copied from gLib implementation.) Instead of treating everything as pointers as gLib does, this custom hasher takes the value of obj_id. Passing obj_id as pointer is just a requirement by function signature (since there may be a need to use other stuff than uint64 as key)

1a1a11a · 2025-05-22T00:57:10Z

libCacheSim/profiler/threadpool.c

+
+#include "../include/libCacheSim/threadpool.h"
+
+#include <pthread.h>


We should move threadpool to utils or dataStructure and add a quick README

1a1a11a · 2025-05-22T00:58:13Z

Yes, you can add a separate section for data structures in doc

mack-w · 2025-05-22T06:55:22Z

Just noted that there seems to be a deadlock problem that I didn't notice while pushing. Trying to fix

In Release builds, access to params->n_caches is optimized to a stack access. In Debug builds however, access is by pointer. Failure to initialize this leads to infinite loop in _simulate, thus deadlock in simulate_* functions.

sonarqubecloud · 2025-05-22T10:55:28Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

mack-w added 5 commits May 15, 2025 02:10

chore: move traceAnalyzer/threadPool.cpp to utils/

1155efd

This commit is the first in a series of commits to remove gLib. This commit moves an unused thread pool library within traceAnalyzer to the utils directory, which may find its use elsewhere.

Revert "chore: move traceAnalyzer/threadPool.cpp to utils/"

bec3b4e

This reverts commit 1155efd.

feat(bin/cachesim): drop GHashTable for header-only hash table lib

41662dc

feat(bin/MRC): drop GHashTable for header-only hash table lib

f9d1c49

mack-w marked this pull request as draft May 15, 2025 17:45

mack-w added 3 commits May 16, 2025 02:20

fix(bin/MRC): struct field naming in commit f9d1c49

6177f2d

chore(include): move redundant defs of obj_id_hasher together

f7d58de

feat(bin/MRC): complete migration to header-only hash table lib

88b7048

mack-w changed the title ~~WIP: remove GLib dependency~~ WIP: remove GLib dependency within *bin/* May 16, 2025

mack-w changed the title WIP: remove GLib dependency within *bin/* WIP: remove GLib dependency within bin/ May 16, 2025

mack-w added 2 commits May 17, 2025 23:18

feat(profiler): remove reference of g_mutex_*

2a9dde1

WIP: feat(profiler): use homebrewed threadpool

9cf2913

Move from GThreadPool to a homebrewed threadpool adapted from https://nachtimwald.com/2019/04/12/thread-pool-in-c Note that this code **only** runs with UBSan enabled. Not for production use (yet).

mack-w added 2 commits May 19, 2025 01:15

fix: clang-format problem in GitHub Actions

7e7c41d

feat(profiler): use homebrewed threadpool

af3ff57

Finalize move from GThreadPool to a homebrewed threadpool adapted from https://nachtimwald.com/2019/04/12/thread-pool-in-c Note, that simulation results differ from original. Reason for this is under investigation.

1a1a11a requested review from 1a1a11a and Copilot May 19, 2025 12:30

Copilot AI reviewed May 19, 2025

View reviewed changes

mack-w marked this pull request as ready for review May 20, 2025 11:08

mack-w marked this pull request as draft May 20, 2025 11:19

fix(simulator): incorrect simulation steps

61fb3cd

The previous commit af3ff57 introduced pthread_cond_t to synchronize progress. However, in some circumstances, the simulate_* functions may exit prematurely due to incorrect conditional wait. This commit fix the abovementioned problem.

mack-w marked this pull request as ready for review May 20, 2025 11:26

mack-w changed the title ~~WIP: remove GLib dependency within bin/~~ Remove GLib dependency within bin/ May 20, 2025

1a1a11a reviewed May 22, 2025

View reviewed changes

mack-w marked this pull request as draft May 22, 2025 06:55

fix(simulator): deadlock in simulate_* functions

f360f63

In Release builds, access to params->n_caches is optimized to a stack access. In Debug builds however, access is by pointer. Failure to initialize this leads to infinite loop in _simulate, thus deadlock in simulate_* functions.

mack-w marked this pull request as ready for review May 22, 2025 08:16

mack-w added 3 commits May 22, 2025 16:26

chore: move threadpool to utils/

5f292f4

doc: some code comments for threadpool.h and hashmap.h

5ebea84

doc: documentation for thread pool and hash table

805efbb


		#include "../include/libCacheSim/threadpool.h"

		#include <pthread.h>

Remove GLib dependency within bin/ #184

Are you sure you want to change the base?

Remove GLib dependency within bin/ #184

Uh oh!

Conversation

mack-w commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1a1a11a commented May 16, 2025

Uh oh!

mack-w commented May 16, 2025

Uh oh!

1a1a11a commented May 17, 2025

Uh oh!

mack-w commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1a1a11a commented May 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI May 19, 2025

Choose a reason for hiding this comment

Uh oh!

mack-w May 22, 2025

Choose a reason for hiding this comment

Uh oh!

mack-w commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mack-w commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1a1a11a May 22, 2025

Choose a reason for hiding this comment

Uh oh!

mack-w May 22, 2025

Choose a reason for hiding this comment

Uh oh!

1a1a11a May 22, 2025

Choose a reason for hiding this comment

Uh oh!

mack-w May 22, 2025

Choose a reason for hiding this comment

Uh oh!

1a1a11a May 22, 2025

Choose a reason for hiding this comment

Uh oh!

mack-w May 22, 2025

Choose a reason for hiding this comment

Uh oh!

1a1a11a commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mack-w commented May 22, 2025

Uh oh!

sonarqubecloud bot commented May 22, 2025

Quality Gate failed

Uh oh!

Uh oh!

mack-w commented May 15, 2025 •

edited

Loading

mack-w commented May 19, 2025 •

edited

Loading

mack-w commented May 19, 2025 •

edited

Loading

mack-w commented May 20, 2025 •

edited

Loading

1a1a11a commented May 22, 2025 •

edited

Loading