protocol: Avoid write locks just to change (now-)atomic booleans

[luke-jr]

No description provided.

[Copilot]

Pull request overview

This PR updates the DATUM protocol job tracking flags so they can be updated without acquiring a write lock, by switching the relevant fields to C11/C23 atomics.

Changes:

• Convert server_has_merkle_branches, server_has_coinbase[*], and server_has_coinbase_empty from bool to atomic_bool.
• Replace rwlock “upgrade-to-write” sections in datum_protocol_pow() with atomic_store_explicit(..., memory_order_relaxed).
• Remove initialization/reset under datum_jobs_rwlock for these flags (since they’re now atomic).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/datum_protocol.h	Switch selected T_DATUM_PROTOCOL_JOB flags to atomic_bool and include <stdatomic.h>.
src/datum_protocol.c	Remove rwlock write-lock upgrades when toggling job flags; use relaxed atomic stores instead; reset flags without rwlock.

Comments suppressed due to low confidence (3)

src/datum_protocol.c:1385

• This relaxed atomic store is good, but the corresponding read (if (!datum_jobs[...].server_has_coinbase_empty)) will be an implicit seq_cst load. Consider using atomic_load_explicit(..., memory_order_relaxed) for that check to avoid stronger-than-needed ordering and extra barriers.

			
			atomic_store_explicit(&datum_jobs[pow->datum_job_id].server_has_coinbase_empty, true, memory_order_relaxed);
		}

src/datum_protocol.c:1398

• The store uses memory_order_relaxed, but the guard condition uses an implicit seq_cst load on the atomic_bool. Consider changing the check to atomic_load_explicit(..., memory_order_relaxed) so the load/store pair uses consistent relaxed semantics and avoids unnecessary ordering costs.

			
			atomic_store_explicit(&datum_jobs[pow->datum_job_id].server_has_coinbase[pow->coinbase_id], true, memory_order_relaxed);
		}

src/datum_protocol.c:1456

• These assignments are now writing to atomic_bool fields, which makes them implicit seq_cst stores. If these flags are intended to be relaxed (as in the atomic_store_explicit(..., memory_order_relaxed) updates), consider initializing/resetting them with atomic_store_explicit(..., false, memory_order_relaxed) for consistency and to avoid stronger-than-needed barriers.

	for(i=0;i<MAX_DATUM_PROTOCOL_JOBS;i++) {
		datum_jobs[i].server_has_merkle_branches = false;
		datum_jobs[i].server_has_coinbase_empty = false;
		for(n=0;n<8;n++) {
			datum_jobs[i].server_has_coinbase[n] = false;
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/datum_protocol.c:1374

• atomic_load_explicit requires the address of the atomic object; this call is missing & and will not compile. Use atomic_load_explicit(&datum_jobs[pow->datum_job_id].server_has_coinbase_empty, …).

		if (!atomic_load_explicit(datum_jobs[pow->datum_job_id].server_has_coinbase_empty, memory_order_relaxed)) {

src/datum_protocol.c:1387

• atomic_load_explicit should be passed a pointer to the atomic element; this call is missing & on server_has_coinbase[pow->coinbase_id] and will fail to compile.

		if (!atomic_load_explicit(datum_jobs[pow->datum_job_id].server_has_coinbase[pow->coinbase_id], memory_order_relaxed)) {

[Copilot]

Pull request overview

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

[kwsantiago]

Dropping this loop's wrlock races the non-atomic memset still done under wrlock in datum_protocol_setup_new_job_idx. keep the lock.