niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

small performance improvement in RHC

Browse source 
The introduction of properly functioning query conditions adds some
overhead, this commit removes some of that cost by avoiding some calls
to update_conditions when there are no query conditions.

It also removes the has_changed field from the instance, instead using a
local boolean to track whether DATA_AVAILABLE should be raised or not.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2019-02-19 10:53:09 +01:00 • committed by eboasson
parent 62a71a870f
commit dd9aceb713
1 changed files with 36 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

72
src/core/ddsc/src/dds_rhc.c
View file

@ -250,7 +250,6 @@ struct rhc_instance {
unsigned isnew : 1; /* NEW or NOT_NEW view state */ unsigned isnew : 1; /* NEW or NOT_NEW view state */
unsigned a_sample_free : 1; /* whether or not a_sample is in use */ unsigned a_sample_free : 1; /* whether or not a_sample is in use */
unsigned isdisposed : 1; /* DISPOSED or NOT_DISPOSED (if not disposed, wrcount determines ALIVE/NOT_ALIVE_NO_WRITERS) */ unsigned isdisposed : 1; /* DISPOSED or NOT_DISPOSED (if not disposed, wrcount determines ALIVE/NOT_ALIVE_NO_WRITERS) */
unsigned has_changed : 1; /* To track changes in an instance - if number of samples are added or data is overwritten */
unsigned wr_iid_islive : 1; /* whether wr_iid is of a live writer */ unsigned wr_iid_islive : 1; /* whether wr_iid is of a live writer */
unsigned inv_exists : 1; /* whether or not state change occurred since last sample (i.e., must return invalid sample) */ unsigned inv_exists : 1; /* whether or not state change occurred since last sample (i.e., must return invalid sample) */
unsigned inv_isread : 1; /* whether or not that state change has been read before */ unsigned inv_isread : 1; /* whether or not that state change has been read before */
@ -313,7 +312,6 @@ struct trigger_info_cmn {
unsigned qminst; unsigned qminst;
bool has_read; bool has_read;
bool has_not_read; bool has_not_read;
bool has_changed;
}; };
struct trigger_info_pre { struct trigger_info_pre {
@ -552,7 +550,7 @@ static void inst_clear_invsample_if_exists (struct rhc *rhc, struct rhc_instance
inst_clear_invsample (rhc, inst, trig_qc); inst_clear_invsample (rhc, inst, trig_qc);
} }
static void inst_set_invsample (struct rhc *rhc, struct rhc_instance *inst, struct trigger_info_qcond *trig_qc) static void inst_set_invsample (struct rhc *rhc, struct rhc_instance *inst, struct trigger_info_qcond *trig_qc, bool *nda)
{ {
if (!inst->inv_exists || inst->inv_isread) if (!inst->inv_exists || inst->inv_isread)
{ {
@ -563,6 +561,7 @@ static void inst_set_invsample (struct rhc *rhc, struct rhc_instance *inst, stru
inst->inv_exists = 1; inst->inv_exists = 1;
inst->inv_isread = 0; inst->inv_isread = 0;
rhc->n_invsamples++; rhc->n_invsamples++;
*nda = true;
} }
} }
@ -651,7 +650,6 @@ static void init_trigger_info_cmn_nonmatch (struct trigger_info_cmn *info)
info->qminst = ~0u; info->qminst = ~0u;
info->has_read = false; info->has_read = false;
info->has_not_read = false; info->has_not_read = false;
info->has_changed = false;
} }
static void get_trigger_info_cmn (struct trigger_info_cmn *info, struct rhc_instance *inst) static void get_trigger_info_cmn (struct trigger_info_cmn *info, struct rhc_instance *inst)
@ -659,12 +657,10 @@ static void get_trigger_info_cmn (struct trigger_info_cmn *info, struct rhc_inst
info->qminst = qmask_of_inst (inst); info->qminst = qmask_of_inst (inst);
info->has_read = inst_has_read (inst); info->has_read = inst_has_read (inst);
info->has_not_read = inst_has_unread (inst); info->has_not_read = inst_has_unread (inst);
info->has_changed = inst->has_changed;
} }
static void get_trigger_info_pre (struct trigger_info_pre *info, struct rhc_instance *inst) static void get_trigger_info_pre (struct trigger_info_pre *info, struct rhc_instance *inst)
{ {
inst->has_changed = 0;
get_trigger_info_cmn (&info->c, inst); get_trigger_info_cmn (&info->c, inst);
} }
@ -680,15 +676,19 @@ static void init_trigger_info_qcond (struct trigger_info_qcond *qc)
qc->inc_conds_sample = 0; qc->inc_conds_sample = 0;
} }
static bool trigger_info_differs (const struct trigger_info_pre *pre, const struct trigger_info_post *post, const struct trigger_info_qcond *trig_qc) static bool trigger_info_differs (const struct rhc *rhc, const struct trigger_info_pre *pre, const struct trigger_info_post *post, const struct trigger_info_qcond *trig_qc)
{ {
return (pre->c.qminst != post->c.qminst || if (pre->c.qminst != post->c.qminst ||
pre->c.has_read != post->c.has_read || pre->c.has_read != post->c.has_read ||
pre->c.has_not_read != post->c.has_not_read || pre->c.has_not_read != post->c.has_not_read)
pre->c.has_changed != post->c.has_changed || return true;
trig_qc->dec_conds_invsample != trig_qc->inc_conds_invsample || else if (rhc->nqconds == 0)
return false;
else
return (trig_qc->dec_conds_invsample != trig_qc->inc_conds_invsample ||
trig_qc->dec_conds_sample != trig_qc->inc_conds_sample || trig_qc->dec_conds_sample != trig_qc->inc_conds_sample ||
trig_qc->dec_invsample_read != trig_qc->inc_invsample_read); trig_qc->dec_invsample_read != trig_qc->inc_invsample_read ||
trig_qc->dec_sample_read != trig_qc->inc_sample_read);
} }
static bool add_sample (struct rhc *rhc, struct rhc_instance *inst, const struct proxy_writer_info *pwr_info, const struct ddsi_serdata *sample, status_cb_data_t *cb_data, struct trigger_info_qcond *trig_qc) static bool add_sample (struct rhc *rhc, struct rhc_instance *inst, const struct proxy_writer_info *pwr_info, const struct ddsi_serdata *sample, status_cb_data_t *cb_data, struct trigger_info_qcond *trig_qc)
@ -722,9 +722,6 @@ static bool add_sample (struct rhc *rhc, struct rhc_instance *inst, const struct
inst->nvread--; inst->nvread--;
rhc->n_vread--; rhc->n_vread--;
} }
/* set a flag to indicate instance has changed to notify data_available since the sample is overwritten */
inst->has_changed = 1;
} }
else else
{ {
@ -1042,7 +1039,7 @@ static int rhc_unregister_isreg_w_sideeffects (struct rhc *rhc, const struct rhc
} }
} }
static int rhc_unregister_updateinst (struct rhc *rhc, struct rhc_instance *inst, const struct proxy_writer_info * __restrict pwr_info, nn_wctime_t tstamp, struct trigger_info_qcond *trig_qc) static int rhc_unregister_updateinst (struct rhc *rhc, struct rhc_instance *inst, const struct proxy_writer_info * __restrict pwr_info, nn_wctime_t tstamp, struct trigger_info_qcond *trig_qc, bool *nda)
{ {
assert (inst->wrcount > 0); assert (inst->wrcount > 0);
@ -1074,7 +1071,7 @@ static int rhc_unregister_updateinst (struct rhc *rhc, struct rhc_instance *inst
care.) */ care.) */
if (inst->latest == NULL || inst->latest->isread) if (inst->latest == NULL || inst->latest->isread)
{ {
inst_set_invsample (rhc, inst, trig_qc); inst_set_invsample (rhc, inst, trig_qc, nda);
update_inst (inst, pwr_info, false, tstamp); update_inst (inst, pwr_info, false, tstamp);
} }
if (!inst->isdisposed) if (!inst->isdisposed)
@ -1096,7 +1093,7 @@ static int rhc_unregister_updateinst (struct rhc *rhc, struct rhc_instance *inst
/* Add invalid samples for transition to no-writers */ /* Add invalid samples for transition to no-writers */
TRACE (",#0,empty,nowriters"); TRACE (",#0,empty,nowriters");
assert (inst_is_empty (inst)); assert (inst_is_empty (inst));
inst_set_invsample (rhc, inst, trig_qc); inst_set_invsample (rhc, inst, trig_qc, nda);
update_inst (inst, pwr_info, false, tstamp); update_inst (inst, pwr_info, false, tstamp);
account_for_empty_to_nonempty_transition (rhc, inst); account_for_empty_to_nonempty_transition (rhc, inst);
inst->wr_iid_islive = 0; inst->wr_iid_islive = 0;
@ -1105,8 +1102,10 @@ static int rhc_unregister_updateinst (struct rhc *rhc, struct rhc_instance *inst
} }
} }
static void dds_rhc_unregister (struct rhc *rhc, struct rhc_instance *inst, const struct proxy_writer_info * __restrict pwr_info, nn_wctime_t tstamp, struct trigger_info_post *post, struct trigger_info_qcond *trig_qc) static bool dds_rhc_unregister (struct rhc *rhc, struct rhc_instance *inst, const struct proxy_writer_info * __restrict pwr_info, nn_wctime_t tstamp, struct trigger_info_post *post, struct trigger_info_qcond *trig_qc)
{ {
bool notify_data_available = false;
/* 'post' always gets set; instance may have been freed upon return. */ /* 'post' always gets set; instance may have been freed upon return. */
TRACE (" unregister:"); TRACE (" unregister:");
if (!rhc_unregister_isreg_w_sideeffects (rhc, inst, pwr_info->iid)) if (!rhc_unregister_isreg_w_sideeffects (rhc, inst, pwr_info->iid))
@ -1114,7 +1113,7 @@ static void dds_rhc_unregister (struct rhc *rhc, struct rhc_instance *inst, cons
/* other registrations remain */ /* other registrations remain */
get_trigger_info_cmn (&post->c, inst); get_trigger_info_cmn (&post->c, inst);
} }
else if (rhc_unregister_updateinst (rhc, inst, pwr_info, tstamp, trig_qc)) else if (rhc_unregister_updateinst (rhc, inst, pwr_info, tstamp, trig_qc, &notify_data_available))
{ {
/* instance dropped */ /* instance dropped */
init_trigger_info_cmn_nonmatch (&post->c); init_trigger_info_cmn_nonmatch (&post->c);
@ -1124,6 +1123,7 @@ static void dds_rhc_unregister (struct rhc *rhc, struct rhc_instance *inst, cons
/* no writers remain, but instance not empty */ /* no writers remain, but instance not empty */
get_trigger_info_cmn (&post->c, inst); get_trigger_info_cmn (&post->c, inst);
} }
return notify_data_available;
} }
static struct rhc_instance *alloc_new_instance (const struct rhc *rhc, const struct proxy_writer_info *pwr_info, struct ddsi_serdata *serdata, struct ddsi_tkmap_instance *tk) static struct rhc_instance *alloc_new_instance (const struct rhc *rhc, const struct proxy_writer_info *pwr_info, struct ddsi_serdata *serdata, struct ddsi_tkmap_instance *tk)
@ -1207,7 +1207,8 @@ static rhc_store_result_t rhc_store_new_instance (struct rhc_instance **out_inst
else else
{ {
if (inst->isdisposed) { if (inst->isdisposed) {
inst_set_invsample (rhc, inst, trig_qc); bool nda_dummy = false;
inst_set_invsample (rhc, inst, trig_qc, &nda_dummy);
} }
} }
@ -1242,6 +1243,7 @@ bool dds_rhc_store (struct rhc * __restrict rhc, const struct proxy_writer_info
rhc_store_result_t stored; rhc_store_result_t stored;
status_cb_data_t cb_data; /* Callback data for reader status callback */ status_cb_data_t cb_data; /* Callback data for reader status callback */
bool delivered = true; bool delivered = true;
bool notify_data_available = false;
TRACE ("rhc_store(%"PRIx64",%"PRIx64" si %x has_data %d:", tk->m_iid, wr_iid, statusinfo, has_data); TRACE ("rhc_store(%"PRIx64",%"PRIx64" si %x has_data %d:", tk->m_iid, wr_iid, statusinfo, has_data);
if (!has_data && statusinfo == 0) if (!has_data && statusinfo == 0)
@ -1282,6 +1284,7 @@ bool dds_rhc_store (struct rhc * __restrict rhc, const struct proxy_writer_info
goto error_or_nochange; goto error_or_nochange;
} }
init_trigger_info_cmn_nonmatch (&pre.c); init_trigger_info_cmn_nonmatch (&pre.c);
notify_data_available = true;
} }
} }
else if (!inst_accepts_sample (rhc, inst, pwr_info, sample, has_data)) else if (!inst_accepts_sample (rhc, inst, pwr_info, sample, has_data))
@ -1300,7 +1303,8 @@ bool dds_rhc_store (struct rhc * __restrict rhc, const struct proxy_writer_info
} }
if (statusinfo & NN_STATUSINFO_UNREGISTER) if (statusinfo & NN_STATUSINFO_UNREGISTER)
{ {
dds_rhc_unregister (rhc, inst, pwr_info, sample->timestamp, &post, &trig_qc); if (dds_rhc_unregister (rhc, inst, pwr_info, sample->timestamp, &post, &trig_qc))
notify_data_available = true;
} }
else else
{ {
@ -1389,11 +1393,12 @@ bool dds_rhc_store (struct rhc * __restrict rhc, const struct proxy_writer_info
inst->disposed_gen--; inst->disposed_gen--;
goto error_or_nochange; goto error_or_nochange;
} }
notify_data_available = true;
} }
/* If instance became disposed, add an invalid sample if there are no samples left */ /* If instance became disposed, add an invalid sample if there are no samples left */
if (inst_became_disposed && inst->latest == NULL) if (inst_became_disposed && inst->latest == NULL)
inst_set_invsample (rhc, inst, &trig_qc); inst_set_invsample (rhc, inst, &trig_qc, &notify_data_available);
update_inst (inst, pwr_info, true, sample->timestamp); update_inst (inst, pwr_info, true, sample->timestamp);
@ -1449,18 +1454,7 @@ bool dds_rhc_store (struct rhc * __restrict rhc, const struct proxy_writer_info
TRACE (")\n"); TRACE (")\n");
const bool update_read_conditions = trigger_info_differs (&pre, &post, &trig_qc); trigger_waitsets = trigger_info_differs (rhc, &pre, &post, &trig_qc) && update_conditions_locked (rhc, true, &pre, &post, &trig_qc, inst);
bool notify_data_available;
/* do not send data available notification when an instance is dropped */
if ((post.c.qminst == ~0u) && (post.c.has_read == 0) && (post.c.has_not_read == 0) && (post.c.has_changed == false))
notify_data_available = false;
else /* FIXME: now that trigger_info_differs incorporates details on samples added/removed, this might well be wrong */
notify_data_available = update_read_conditions;
if (update_read_conditions)
trigger_waitsets = update_conditions_locked (rhc, true, &pre, &post, &trig_qc, inst);
else
trigger_waitsets = false;
assert (rhc_check_counts_locked (rhc, true, true)); assert (rhc_check_counts_locked (rhc, true, true));
@ -1557,7 +1551,7 @@ void dds_rhc_unregister_wr (struct rhc * __restrict rhc, const struct proxy_writ
else else
{ {
const bool was_empty = inst_is_empty (inst); const bool was_empty = inst_is_empty (inst);
inst_set_invsample (rhc, inst, &trig_qc); inst_set_invsample (rhc, inst, &trig_qc, &notify_data_available);
if (was_empty) if (was_empty)
account_for_empty_to_nonempty_transition (rhc, inst); account_for_empty_to_nonempty_transition (rhc, inst);
else else
@ -1570,7 +1564,7 @@ void dds_rhc_unregister_wr (struct rhc * __restrict rhc, const struct proxy_writ
TRACE ("\n"); TRACE ("\n");
notify_data_available = true; notify_data_available = true;
if (trigger_info_differs (&pre, &post, &trig_qc) && update_conditions_locked (rhc, true, &pre, &post, &trig_qc, inst)) if (trigger_info_differs (rhc, &pre, &post, &trig_qc) && update_conditions_locked (rhc, true, &pre, &post, &trig_qc, inst))
trigger_waitsets = true; trigger_waitsets = true;
assert (rhc_check_counts_locked (rhc, true, false)); assert (rhc_check_counts_locked (rhc, true, false));
} }
@ -1589,7 +1583,9 @@ void dds_rhc_unregister_wr (struct rhc * __restrict rhc, const struct proxy_writ
} }
if (trigger_waitsets) if (trigger_waitsets)
dds_entity_status_signal (&rhc->reader->m_entity); {
dds_entity_status_signal(&rhc->reader->m_entity);
}
} }
} }