Skip to content

Commit 1341ce3

Browse files
bors[bot]cuviper
andauthored
1025: Stop deriving traits on JobRef r=cuviper a=cuviper I don't see any difference in benchmarks from this, and only a slight reduction in code size. If nothing else I think it's more principled that `JobRef` should not implement `Copy` in particular. `Registry::inject` taking a slice of jobs was an over-abstraction that we never used. Co-authored-by: Josh Stone <cuviper@gmail.com>
2 parents 4a2de59 + b6cc378 commit 1341ce3

File tree

5 files changed

+22
-19
lines changed

5 files changed

+22
-19
lines changed

rayon-core/src/job.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ pub(super) trait Job {
3030
/// Internally, we store the job's data in a `*const ()` pointer. The
3131
/// true type is something like `*const StackJob<...>`, but we hide
3232
/// it. We also carry the "execute fn" from the `Job` trait.
33-
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
3433
pub(super) struct JobRef {
3534
pointer: *const (),
3635
execute_fn: unsafe fn(*const ()),
@@ -74,6 +73,17 @@ where
7473
result: UnsafeCell<JobResult<R>>,
7574
}
7675

76+
impl<L, F, R> PartialEq<JobRef> for StackJob<L, F, R>
77+
where
78+
L: Latch + Sync,
79+
F: FnOnce(bool) -> R + Send,
80+
R: Send,
81+
{
82+
fn eq(&self, job: &JobRef) -> bool {
83+
job.pointer == <*const Self>::cast(self) && job.execute_fn == Self::execute
84+
}
85+
}
86+
7787
impl<L, F, R> StackJob<L, F, R>
7888
where
7989
L: Latch + Sync,

rayon-core/src/join/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,7 @@ where
134134
// done here so that the stack frame can keep it all live
135135
// long enough.
136136
let job_b = StackJob::new(call_b(oper_b), SpinLatch::new(worker_thread));
137-
let job_b_ref = job_b.as_job_ref();
138-
worker_thread.push(job_b_ref);
137+
worker_thread.push(job_b.as_job_ref());
139138

140139
// Execute task a; hopefully b gets stolen in the meantime.
141140
let status_a = unwind::halt_unwinding(call_a(oper_a, injected));
@@ -151,7 +150,7 @@ where
151150
// those off to get to it.
152151
while !job_b.latch.probe() {
153152
if let Some(job) = worker_thread.take_local_job() {
154-
if job == job_b_ref {
153+
if job_b == job {
155154
// Found it! Let's run it.
156155
//
157156
// Note that this could panic, but it's ok if we unwind here.

rayon-core/src/registry.rs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -416,18 +416,16 @@ impl Registry {
416416
if !worker_thread.is_null() && (*worker_thread).registry().id() == self.id() {
417417
(*worker_thread).push(job_ref);
418418
} else {
419-
self.inject(&[job_ref]);
419+
self.inject(job_ref);
420420
}
421421
}
422422
}
423423

424424
/// Push a job into the "external jobs" queue; it will be taken by
425425
/// whatever worker has nothing to do. Use this if you know that
426426
/// you are not on a worker of this registry.
427-
pub(super) fn inject(&self, injected_jobs: &[JobRef]) {
428-
self.log(|| JobsInjected {
429-
count: injected_jobs.len(),
430-
});
427+
pub(super) fn inject(&self, injected_job: JobRef) {
428+
self.log(|| JobsInjected { count: 1 });
431429

432430
// It should not be possible for `state.terminate` to be true
433431
// here. It is only set to true when the user creates (and
@@ -442,12 +440,8 @@ impl Registry {
442440

443441
let queue_was_empty = self.injected_jobs.is_empty();
444442

445-
for &job_ref in injected_jobs {
446-
self.injected_jobs.push(job_ref);
447-
}
448-
449-
self.sleep
450-
.new_injected_jobs(usize::MAX, injected_jobs.len() as u32, queue_was_empty);
443+
self.injected_jobs.push(injected_job);
444+
self.sleep.new_injected_jobs(usize::MAX, 1, queue_was_empty);
451445
}
452446

453447
fn has_injected_job(&self) -> bool {
@@ -547,7 +541,7 @@ impl Registry {
547541
},
548542
LatchRef::new(l),
549543
);
550-
self.inject(&[job.as_job_ref()]);
544+
self.inject(job.as_job_ref());
551545
job.latch.wait_and_reset(); // Make sure we can use the same latch again next time.
552546

553547
// flush accumulated logs as we exit the thread
@@ -575,7 +569,7 @@ impl Registry {
575569
},
576570
latch,
577571
);
578-
self.inject(&[job.as_job_ref()]);
572+
self.inject(job.as_job_ref());
579573
current_thread.wait_until(&job.latch);
580574
job.into_result()
581575
}

rayon-core/src/scope/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ impl<'scope> ScopeFifo<'scope> {
615615
// SAFETY: this job will execute before the scope ends.
616616
unsafe { worker.push(fifo.push(job_ref)) };
617617
}
618-
None => self.base.registry.inject(&[job_ref]),
618+
None => self.base.registry.inject(job_ref),
619619
}
620620
}
621621

rayon-core/src/spawn/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ where
154154
// in a locally-FIFO order. Otherwise, just use the pool's global injector.
155155
match registry.current_thread() {
156156
Some(worker) => worker.push_fifo(job_ref),
157-
None => registry.inject(&[job_ref]),
157+
None => registry.inject(job_ref),
158158
}
159159
mem::forget(abort_guard);
160160
}

0 commit comments

Comments
 (0)