Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various XHA improvements #18

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

edwintorok
Copy link
Contributor

Might need splitting into separate PRs (there are about 8 independent fixes here). For now keep them together until I test them.

Copy link

@andyhhp andyhhp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you do want to split the mechanical fixes from the logical fixes (esp the lock contention one).

SM_PHASE p1,
SM_PHASE p2,
MTC_BOOLEAN on_heartbeat,
MTC_BOOLEAN on_statefile)
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to de-indent the new rendezvous_wait() too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a separate commit that deindents these

MTC_BOOLEAN writable)
{
{
PCOM_DATA_SF psf;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, where you've de-indented the braces but not the rest of the function

@edwintorok edwintorok force-pushed the private/edvint/lockinglatency branch 5 times, most recently from b96eb51 to c2098f5 Compare February 19, 2025 10:24
Reduce the scope of the global com mutex, otherwise every attempt to
acquire a reader lock results in contention on this global mutex.

Use a per-object-table lock instead.

Signed-off-by: Edwin Török <[email protected]>
Nested functions are a GCC-only extension and are not supported by clang
and clangd at all.

Signed-off-by: Edwin Török <[email protected]>
TODO: use pthread_condattr_setclock and switch to a monotonic clock
instead

Signed-off-by: Edwin Török <[email protected]>
@edwintorok edwintorok force-pushed the private/edvint/lockinglatency branch from c2098f5 to 446f61e Compare February 21, 2025 19:01
@edwintorok
Copy link
Contributor Author

edwintorok commented Feb 21, 2025

I think you do want to split the mechanical fixes from the logical fixes (esp the lock contention one).

Split into:

@@ -937,6 +950,7 @@
{
log_message(MTC_LOG_WARNING, "COM: pthread_rwlock_destroy failed (sys %d).\n", pthread_ret);
}
pthread_ret = pthread_mutex_destroy(&object->thread_id_record_table_mutex);

Check warning

Code scanning / CodeChecker

Value stored to 'pthread_ret' is never read Warning

Value stored to 'pthread_ret' is never read

if (timeout == 0)
{
return FALSE;
}

struct timespec timeout_ts = mstots(timeout);
clock_gettime(CLOCK_REALTIME, &deadline);

Check warning

Code scanning / CodeChecker

the value returned by this function should not be disregarded; neglecting it may lead to errors Warning

the value returned by this function should not be disregarded; neglecting it may lead to errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants