huntedbytheirs/fastwc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: thread-safety in count_sliced, ship measured wins

Browse Source 
Review (5-lane) found one MAJOR: count_sliced ignored pthread_create's
return value - on EAGAIN it joined an indeterminate pthread_t and summed
an uninitialized slice, UB plus a silently wrong count. Threads are now
initialized to 0, a failed create counts its slice inline, and nt is
hard-capped at a named MAX_THREADS (the fixed jobs[8]/th[8] arrays
smash the stack past 8 threads - reproduced by QA's thread sweep).

Shipped from the measured optimization hunt:
- -w-only mode skips the newline compare/popcount entirely (the
  (x-9)<5 range already covers '\n' in the whitespace mask) - a
  need_lines gate threads through every kernel, the scalar reference,
  the avx512 mirror, and the sliced workers.
- Thread retune: 12 threads past 32 MiB, 16 past 256 MiB (was 8) -
  up to 18% on 110 MB, ~3% on 1.1 GB in QA's interleaved sweep.
- Benchmark suites pin LC_ALL=C so GNU wc -w can't silently switch to
  multibyte decoding and inflate the win.
- popcount16 is guarded to x86 builds (zero-warnings on other arches);
  checkwc's stray argument dropped.

Verified: selftest (kernels + sliced, now with need_lines coverage),
120-trial fuzz vs GNU (file + stdin, both locales), full test-all suite
three consecutive times, format-check and clang-tidy clean. The 1B
solo monster now lands at 3.7-6s (up to 268 Mlines/s) depending on how
warm the page cache is feeling.
This commit is contained in:
huntedbytheirs
2026-08-29 17:03:31 -04:00
parent 6aa461f053
commit ae5068d4e7
7 changed files with 120 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main.c
+102 -56
View File
@@ -146,6 +146,7 @@ static long long count_words(const unsigned char *s, size_t n, int *prev_ws)
return w;
}
#if defined(__x86_64__) || defined(__i386__)
/* SSE2 predates POPCNT; count 16-bit masks with the classic bit trick. */
static unsigned popcount16(unsigned x)
{
@@ -154,6 +155,7 @@ static unsigned popcount16(unsigned x)
x = (x + (x >> 4)) & 0x0f0f;
return (x + (x >> 8)) & 0xff;
}
#endif /* x86 */
typedef struct
{
@@ -162,7 +164,7 @@ typedef struct
} lw_t;
typedef lw_t (*count_lw_fn)(const unsigned char *s, size_t n, int *prev_ws,
int need_words);
int need_lines, int need_words);
/*
* Word separators match GNU wc (the benchmark oracle): the six C-locale
@@ -172,7 +174,8 @@ typedef lw_t (*count_lw_fn)(const unsigned char *s, size_t n, int *prev_ws,
#if defined(__x86_64__) || defined(__i386__)
__attribute__((target("avx512f,avx512bw"))) static lw_t
count_lw_avx512(const unsigned char *s, size_t n, int *prev_ws, int need_words)
count_lw_avx512(const unsigned char *s, size_t n, int *prev_ws, int need_lines,
int need_words)
{
const __m512i nl = _mm512_set1_epi8('\n');
const __m512i sp = _mm512_set1_epi8(' ');
@@ -186,13 +189,18 @@ count_lw_avx512(const unsigned char *s, size_t n, int *prev_ws, int need_words)
for (; i + 64 <= n; i += 64)
{
__m512i v = _mm512_loadu_si512((const void *)(s + i));
uint64_t nl_mask = (uint64_t)_mm512_cmpeq_epi8_mask(v, nl);
uint64_t nl_mask = 0;
lines += (long long)_mm_popcnt_u64(nl_mask);
if (need_lines)
{
nl_mask = (uint64_t)_mm512_cmpeq_epi8_mask(v, nl);
lines += (long long)_mm_popcnt_u64(nl_mask);
}
if (need_words)
{
/* min(d, 4) == d <=> (x - 9) < 5 unsigned */
/* min(d, 4) == d <=> (x - 9) < 5 unsigned; the range
* already covers '\n', so no newline compare is needed */
__m512i d = _mm512_sub_epi8(v, lo);
uint64_t ws =
nl_mask | (uint64_t)_mm512_cmpeq_epi8_mask(v, sp) |
@@ -206,7 +214,8 @@ count_lw_avx512(const unsigned char *s, size_t n, int *prev_ws, int need_words)
for (; i < n; i++)
{
int ws = ws_tab[s[i]];
lines += s[i] == '\n';
if (need_lines)
lines += s[i] == '\n';
if (need_words)
{
if (prev && !ws)
@@ -221,7 +230,8 @@ count_lw_avx512(const unsigned char *s, size_t n, int *prev_ws, int need_words)
}
__attribute__((target("avx2"))) static lw_t
count_lw_avx2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
count_lw_avx2(const unsigned char *s, size_t n, int *prev_ws, int need_lines,
int need_words)
{
const __m256i nl = _mm256_set1_epi8('\n');
const __m256i sp = _mm256_set1_epi8(' ');
@@ -235,14 +245,18 @@ count_lw_avx2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
for (; i + 32 <= n; i += 32)
{
__m256i v = _mm256_loadu_si256((const void *)(s + i));
uint32_t nl_mask =
(uint32_t)_mm256_movemask_epi8(_mm256_cmpeq_epi8(v, nl));
uint32_t nl_mask = 0;
lines += (long long)_mm_popcnt_u32(nl_mask);
if (need_lines)
{
nl_mask = (uint32_t)_mm256_movemask_epi8(_mm256_cmpeq_epi8(v, nl));
lines += (long long)_mm_popcnt_u32(nl_mask);
}
if (need_words)
{
/* min(d, 4) == d <=> (x - 9) < 5 unsigned */
/* min(d, 4) == d <=> (x - 9) < 5 unsigned; the range
* already covers '\n', so no newline compare is needed */
__m256i d = _mm256_sub_epi8(v, lo);
uint32_t ws =
nl_mask |
@@ -258,7 +272,8 @@ count_lw_avx2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
for (; i < n; i++)
{
int ws = ws_tab[s[i]];
lines += s[i] == '\n';
if (need_lines)
lines += s[i] == '\n';
if (need_words)
{
if (prev && !ws)
@@ -273,7 +288,8 @@ count_lw_avx2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
}
__attribute__((target("sse2"))) static lw_t
count_lw_sse2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
count_lw_sse2(const unsigned char *s, size_t n, int *prev_ws, int need_lines,
int need_words)
{
const __m128i nl = _mm_set1_epi8('\n');
const __m128i sp = _mm_set1_epi8(' ');
@@ -287,9 +303,13 @@ count_lw_sse2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
for (; i + 16 <= n; i += 16)
{
__m128i v = _mm_loadu_si128((const void *)(s + i));
uint32_t nl_mask = (uint32_t)_mm_movemask_epi8(_mm_cmpeq_epi8(v, nl));
uint32_t nl_mask = 0;
lines += (long long)popcount16(nl_mask);
if (need_lines)
{
nl_mask = (uint32_t)_mm_movemask_epi8(_mm_cmpeq_epi8(v, nl));
lines += (long long)popcount16(nl_mask);
}
if (need_words)
{
@@ -307,7 +327,8 @@ count_lw_sse2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
for (; i < n; i++)
{
int ws = ws_tab[s[i]];
lines += s[i] == '\n';
if (need_lines)
lines += s[i] == '\n';
if (need_words)
{
if (prev && !ws)
@@ -325,11 +346,11 @@ count_lw_sse2(const unsigned char *s, size_t n, int *prev_ws, int need_words)
/* Reference path: the two scalar SWAR counters, kept as the fallback. */
static lw_t count_lw_scalar(const unsigned char *s, size_t n, int *prev_ws,
int need_words)
int need_lines, int need_words)
{
lw_t r;
r.lines = count_newlines(s, n);
r.lines = need_lines ? count_newlines(s, n) : 0;
r.words = need_words ? count_words(s, n, prev_ws) : 0;
return r;
}
@@ -513,7 +534,8 @@ static void count_stream(FILE *fp, counts_t *c)
if (flags & (F_LINES | F_WORDS))
{
lw_t r =
count_lw(buf, nread, &prev_ws, (flags & F_WORDS) != 0);
count_lw(buf, nread, &prev_ws, (flags & F_LINES) != 0,
(flags & F_WORDS) != 0);
if (flags & F_LINES)
c->lines += r.lines;
if (flags & F_WORDS)
@@ -560,7 +582,8 @@ static void count_stream(FILE *fp, counts_t *c)
c->bytes += (long long)nread;
if (flags & (F_LINES | F_WORDS))
{
lw_t r = count_lw(buf, nread, &prev_ws, (flags & F_WORDS) != 0);
lw_t r = count_lw(buf, nread, &prev_ws, (flags & F_LINES) != 0,
(flags & F_WORDS) != 0);
if (flags & F_LINES)
c->lines += r.lines;
if (flags & F_WORDS)
@@ -577,6 +600,7 @@ typedef struct
const unsigned char *s;
size_t n;
int prev_ws;
int need_lines;
int need_words;
lw_t r;
} mjob_t;
@@ -585,7 +609,7 @@ static void *map_worker(void *arg)
{
mjob_t *j = arg;
j->r = count_lw(j->s, j->n, &j->prev_ws, j->need_words);
j->r = count_lw(j->s, j->n, &j->prev_ws, j->need_lines, j->need_words);
return NULL;
}
@@ -596,14 +620,21 @@ static void *map_worker(void *arg)
* which makes the split exact. The kernels are pure, so no locks.
*/
static void count_sliced(const unsigned char *p, size_t n, int nt,
int need_words, long long *lines, long long *words)
int need_lines, int need_words, long long *lines,
long long *words)
{
mjob_t jobs[8];
pthread_t th[8];
enum
{
MAX_THREADS = 16
};
mjob_t jobs[MAX_THREADS];
pthread_t th[MAX_THREADS];
long long tl = 0, tw = 0;
size_t per = (n + (size_t)nt - 1) / (size_t)nt;
int i;
if (nt > MAX_THREADS)
nt = MAX_THREADS;
per = (per + 63) & ~(size_t)63;
if (per == 0)
per = 64;
@@ -615,11 +646,18 @@ static void count_sliced(const unsigned char *p, size_t n, int nt,
jobs[i].s = p + start;
jobs[i].n = (start + per <= n) ? per : (start < n ? n - start : 0);
jobs[i].prev_ws = (i == 0 || jobs[i].n == 0) ? 1 : ws_tab[p[start - 1]];
jobs[i].need_lines = need_lines;
jobs[i].need_words = need_words;
th[i] = 0;
if (jobs[i].n > 0)
pthread_create(&th[i], NULL, map_worker, &jobs[i]);
else
th[i] = 0;
{
if (pthread_create(&th[i], NULL, map_worker, &jobs[i]) != 0)
{ /* out of threads: count this slice inline rather than lose it */
map_worker(&jobs[i]);
tl += jobs[i].r.lines;
tw += jobs[i].r.words;
}
}
}
for (i = 0; i < nt; i++)
@@ -640,8 +678,10 @@ static int pick_threads(size_t n)
long ncpu = sysconf(_SC_NPROCESSORS_ONLN);
int nt;
if (n >= (size_t)32 << 20)
nt = 8;
if (n >= (size_t)256 << 20)
nt = 16;
else if (n >= (size_t)32 << 20)
nt = 12;
else if (n >= (size_t)8 << 20)
nt = 4;
else
@@ -653,6 +693,7 @@ static int pick_threads(size_t n)
static void count_mapped(const unsigned char *p, size_t n, counts_t *c)
{
int need_lines = (flags & F_LINES) != 0;
int need_words = (flags & F_WORDS) != 0;
int nt = pick_threads(n);
long long lines = 0, words = 0;
@@ -662,14 +703,14 @@ static void count_mapped(const unsigned char *p, size_t n, counts_t *c)
if (nt <= 1)
{
int prev_ws = 1;
lw_t r = count_lw(p, n, &prev_ws, need_words);
lw_t r = count_lw(p, n, &prev_ws, need_lines, need_words);
lines = r.lines;
words = r.words;
}
else
{
count_sliced(p, n, nt, need_words, &lines, &words);
count_sliced(p, n, nt, need_lines, need_words, &lines, &words);
}
if (flags & F_LINES)
@@ -915,7 +956,7 @@ static long long ref_words(const unsigned char *s, size_t n, int *prev_ws)
* the algorithm; this proves the width.
*/
static lw_t count_lw_avx512_mirror(const unsigned char *s, size_t n,
int *prev_ws, int need_words)
int *prev_ws, int need_lines, int need_words)
{
long long lines = 0, words = 0;
size_t i = 0;
@@ -931,7 +972,8 @@ static lw_t count_lw_avx512_mirror(const unsigned char *s, size_t n,
if (ws_tab[s[i + j]])
ws |= (uint64_t)1 << j;
}
lines += (long long)__builtin_popcountll(nl_mask);
if (need_lines)
lines += (long long)__builtin_popcountll(nl_mask);
if (need_words)
{
words += (long long)__builtin_popcountll(~ws & ((ws << 1) | prev));
@@ -942,7 +984,8 @@ static lw_t count_lw_avx512_mirror(const unsigned char *s, size_t n,
for (; i < n; i++)
{
int ws = ws_tab[s[i]];
lines += s[i] == '\n';
if (need_lines)
lines += s[i] == '\n';
if (need_words)
{
if (prev && !ws)
@@ -968,29 +1011,32 @@ static int check_kernel(const char *name, count_lw_fn fn)
buf[k] = (unsigned char)rng32();
for (int pw = 0; pw <= 1; pw++)
{
for (int nw = 0; nw <= 1; nw++)
for (int nl = 0; nl <= 1; nl++)
{
int a = pw, b = pw;
lw_t got = fn(buf, n, &a, nw);
long long want_l = ref_lines(buf, n);
long long want_w = nw ? ref_words(buf, n, &b) : 0;
if (got.lines != want_l || got.words != want_w ||
a != (nw ? b : pw))
for (int nw = 0; nw <= 1; nw++)
{
printf("%s: n=%zu pw=%d nw=%d lines %lld/%lld "
"words %lld/%lld state %d/%d\n",
name, n, pw, nw, got.lines, want_l, got.words,
want_w, a, b);
if (n <= 64)
int a = pw, b = pw;
lw_t got = fn(buf, n, &a, nl, nw);
long long want_l = nl ? ref_lines(buf, n) : 0;
long long want_w = nw ? ref_words(buf, n, &b) : 0;
if (got.lines != want_l || got.words != want_w ||
a != (nw ? b : pw))
{
for (k = 0; k < n; k++)
printf("%02x", buf[k]);
printf("\n");
printf("%s: n=%zu pw=%d nl=%d nw=%d lines %lld/%lld "
"words %lld/%lld state %d/%d\n",
name, n, pw, nl, nw, got.lines, want_l,
got.words, want_w, a, b);
if (n <= 64)
{
for (k = 0; k < n; k++)
printf("%02x", buf[k]);
printf("\n");
}
fails++;
if (fails > 5)
return fails;
}
fails++;
if (fails > 5)
return fails;
}
}
}
@@ -1004,7 +1050,7 @@ static int check_kernel(const char *name, count_lw_fn fn)
for (int pw = 0; pw <= 1; pw++)
{
int a = pw, b = pw;
lw_t got = fn(buf, n, &a, 1);
lw_t got = fn(buf, n, &a, 1, 1);
if (got.lines != ref_lines(buf, n) ||
got.words != ref_words(buf, n, &b) || a != b)
@@ -1045,7 +1091,7 @@ static int check_sliced(void)
int pw = 1;
long long want_w = count_words(buf, n, &pw);
count_sliced(buf, n, nt, 1, &tl, &tw);
count_sliced(buf, n, nt, 1, 1, &tl, &tw);
if (tl != want_l || tw != want_w)
{
printf("sliced: n=%zu nt=%d lines %lld/%lld "
@@ -1070,7 +1116,7 @@ static int check_sliced(void)
int pw = 1;
long long want_w = count_words(buf, n, &pw);
count_sliced(buf, n, nt, 1, &tl, &tw);
count_sliced(buf, n, nt, 1, 1, &tl, &tw);
if (tl != want_l || tw != want_w)
{
printf("sliced-ws: n=%zu nt=%d lines %lld/%lld "