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

libCacheSim/profiler/simulator.c

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.

libCacheSim/bin/cli_reader_utils.c

libCacheSim/profiler/threadpool.c

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.

doc/datastructures.md

libCacheSim/bin/MRC/SHARDS.c

1a1a11a · 2025-05-25T18:53:54Z

libCacheSim/include/libCacheSim/hashmap.h

[optional] it would be good to have a benchmark for hash table.

Shall figure out a way to do so
Does it need to be a benchmark on libCacheSim, or a independent/use case-irrelevant benchmark?

An independent benchmark should be sufficient, and it would be good to compare with a common library, e.g., Glib, C++ std, or absl hashtable. BTW, I am also creating a new issue to build a performance test for libCacheSim.

work in progress
@1a1a11a Where shall I put the benckmark? As a test program or just detail in the docs

libCacheSim/profiler/dist.c

libCacheSim/profiler/simulator.c

mack-w · 2025-05-30T05:13:25Z

@1a1a11a Sorry for not working this week, having some personal issues that I have to fix. Shall resume next week

Includes comments cleanup, formatting issues, FIXME notes on conversion macros (in next commit), spelling, etc.

1a1a11a · 2025-06-03T14:00:42Z

You can create a separate benchmark folder in the top directory, i.e., benchmarks/dataStructure/, would this work?

mack-w · 2025-06-03T15:04:54Z

That makes sense considering that you want to add benchmark to the simulator.
Anyways I'm putting this as second in priority list, maybe I will open a separate PR for this :)

1a1a11a · 2025-06-13T23:11:57Z

Shall I take a look?

mack-w · 2025-06-14T03:53:11Z

Shall I take a look?

Yeah I consider this PR "finished" (except for the benchmark part)
@1a1a11a Sorry I really had a lot of personal issues recently... I think next week I'll finally be able to look into smth with focus.

1a1a11a · 2025-06-16T23:34:52Z

@haochengxia Can you take a look at this PR?

sonarqubecloud · 2025-06-16T23:35:40Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

haochengxia · 2025-06-17T00:45:30Z

libCacheSim/profiler/dist.c

@@ -85,7 +91,7 @@ int64_t get_stack_dist_add_req(const request_t *req, sTree **splay_tree,
    newtree = insert(curr_ts, *splay_tree);
  } else {
    // not first time access


When converting a pointer to an integer, can we use int64_t old_ts = (int64_t)(uintptr_t)gp; to avoid potential issues across archs?

BTW, as we removed glib, is gp still a good variable name?

haochengxia · 2025-06-17T01:00:12Z

libCacheSim/include/conversion.h

+#define cptr_t_ const void*
+
+// platform-depedent
+#define ptr_size_t_ size_t


It is now quite good, and I recommend using stdint.h for robustness and readability.

e.g.,
#define ptr_uintptr_t_ uintptr_t
#define ptr_intptr_t_ intptr_t

#define int_to_ptr(i) (ptr_t_)((ptr_intptr_t_)i)
#define uint_to_ptr(i) (ptr_t_)((ptr_uintptr_t_)i)
#define int_to_cptr(i) (cptr_t_)((ptr_intptr_t_)i)
#define uint_to_cptr(i) (cptr_t_)((ptr_uintptr_t_)i)

#define ptr_to_int(p) (int)((ptr_intptr_t_)p)
#define ptr_to_uint(p) (unsigned int)((ptr_uintptr_t_)p)
...

haochengxia · 2025-06-17T15:14:58Z

libCacheSim/profiler/profilerLRU.c

 #include "../include/libCacheSim/profilerLRU.h"

+#include "../dataStructure/splay.h"


We can preserve the order of header files.

I'd like to preserve the order, but the current order is determined by clang-format.

mack-w · 2025-06-19T07:42:32Z

@haochengxia Thanks for the code suggestions.
Also I see there's merge conflict. I will fix that and update my own branch soon.

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

libCacheSim/traceReader/reader.c Show resolved Hide resolved

libCacheSim/profiler/simulator.c Outdated Show resolved Hide resolved

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

libCacheSim/bin/cli_reader_utils.c Show resolved Hide resolved

1a1a11a reviewed May 22, 2025

View reviewed changes

libCacheSim/bin/cli_reader_utils.c Outdated Show resolved Hide resolved

libCacheSim/profiler/threadpool.c Outdated Show resolved Hide resolved

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

1a1a11a reviewed May 25, 2025

View reviewed changes

mack-w marked this pull request as draft May 30, 2025 05:13

mack-w added 5 commits June 1, 2025 22:38

fix: clang-format problem (again) in GitHub Actions

83e1c93

fix: various issues raised within PR comments

ef86c83

Includes comments cleanup, formatting issues, FIXME notes on conversion macros (in next commit), spelling, etc.

fix: didn't include hashmap_defs.h in prev commit ef86c83

26b4a00

chore: introduce macro for common conversions e.g. int to pointer

760bbca

fix: didn't include cli_reader_utils in prev commit

64a0fde

mack-w marked this pull request as ready for review June 3, 2025 08:33

Merge branch 'develop' into develop

c135155

haochengxia mentioned this pull request Jun 17, 2025

Should we consider goto-based error handling? #194

Closed

haochengxia reviewed Jun 17, 2025

View reviewed changes

		#include "../include/libCacheSim/profilerLRU.h"

		#include "../dataStructure/splay.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!

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!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

1a1a11a May 25, 2025

Choose a reason for hiding this comment

Uh oh!

mack-w May 26, 2025

Choose a reason for hiding this comment

Uh oh!

1a1a11a May 26, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mack-w commented May 30, 2025

Uh oh!

1a1a11a commented Jun 3, 2025

Uh oh!

mack-w commented Jun 3, 2025

Uh oh!

1a1a11a commented Jun 13, 2025

Uh oh!

mack-w commented Jun 14, 2025

Uh oh!

1a1a11a commented Jun 16, 2025

Uh oh!

sonarqubecloud bot commented Jun 16, 2025

Quality Gate failed

Uh oh!

haochengxia Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

haochengxia Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

haochengxia Jun 17, 2025

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

mack-w Jun 3, 2025 •

edited

Loading