Thomas Munro <thomas.munro@enterprisedb.com> writes:
> One very small thing after another look:
> - Assert(cv_sleep_target == NULL);
> + if (cv_sleep_target != NULL)
> + ConditionVariableCancelSleep();
> The test for cv_sleep_target != NULL is redundant since
> ConditionVariableCancelSleep() would return early.
True. I did that because Robert was already objecting to the cost
of an added test-and-branch here, so I figured he'd be really
unhappy with the cost of a function call plus test-and-branch ;-)
> ConditionVariableBroadcast() doesn't do that.
Yup. I considered removing the discrepancy by adding a similar
if-guard in ConditionVariableBroadcast(). The internal test in
ConditionVariableCancelSleep would then be only for the benefit
of outside callers such as AbortTransaction, but that seems fine
and per its documentation.
Or we could remove those if's and save a few bytes at the
cost of some cycles. I don't care much.
regards, tom lane