Re: pcp updates: libpcp multithreading - documentation and tools for call graphs & lock analysis

Hi, Ken -

> This is very much WIP.

It is a great christmas present, knowing that the topic is alive for
discussion again and that you're engaged!

I scanned through your ODT writeup, but not through the (clever!)
static-analysis graphing machinery, and have a bunch of comments.


Firat, an important point regarding the footnote on page 1.  It's
really not clear that single-threaded programs inherently place less
load on a system than multi-threaded ones.  Actually, one bothers with
the effort toward threading in order to reduce latency, not to
increase workload.  Indeed, an efficient single-threaded program doing
even a CPU-bound calculation will take **fewer** overall CPU cycles
than if performing the same calculation in parallel across the CPUs.
It's only elapsed time that's improved with threading.  And latency -
elapsed time - is our serious enemy in PCP.


Page 2, regarding the Big Lock.  I believe that we are covering far
too many individual critical sections with a single lock.  Many of the
data structures that it protects could be handled with much smaller
lifespan local locks.  Other than the lock-order-inversion problems,
at the least the Big Lock impedes concurrency.


Page 3, regarding __pmLock_libpcp.  I believe that making this lock
recursive was both an unfortunate and unnecessary design choice.  It's
unfortunate because it enables poor/incomplete design elsewhere (as it
has in libpcp), and it's unnecessary because we could do better.

Regarding the unfortunate side, please read
http://www.zaval.org/resources/library/butenhof1.html about recursive
mutexes, and let it soak in.

Regarding the unnecessary side, if we drew a careful line at the PMAPI
level, that incoming entry points into the shared library functions
perform the necessary locking, and internal functions presume locking
to have been done, we could do away with the recursion.  For example,
one simple scheme have the pm* functions take any locks they require,
then call __pm* siblings.  Forbid __pm* ones from calling locking pm*
ones.  

Have __pm* ones assert incoming lockedness; have valgrind/helgrind
confirm.  (Our unfortunate inconsistent naming conventions about
"internal" __-functions being also exposed in the PMAPI /
shared-library will come back to bite us, in which case we may need a
different naming convention for locking vs presumed-locked function
pairs.)

Regarding the __pmHandleToPtr c_lock vs. libpcp lock comments on page
three, hallelujah, I'm glad we agree that this lookup race needs to be
closed.

Regarding the IPC-with-PMCD serialization, I still believe that
dropping the socket sharing between multiple contexts to the same PMCD
is the most robust and also easiest way to handle this whole area.  It
would eliminate the pc_lock entirely.  It would enable us to benefit
from a future concurrent-capable PMCD.  It would make HOST pmContexts
completely independent.  It would cost us a few more FDs, but so what.


Regarding PM_CONTEXT_LOCAL on page 4, this is not too serious, but I
am pretty sure we could permit multithreaded apps to call into the
PMAPI even with such contexts.  We'd just have to serialize within
libpcp at the PMDA DSO call-out level.  In case we have PMDAs that
just cannot tolerate being called into from different threads (even
serialized one at a time), those PMDAs could trigger those
PM_MULTIPLE_THREADS errors themselves.  It need not be a systemic
obstacle.

The __pmAcc* stuff could use a rework to avoid global variables.
Really, we need ACLs to be contextual rather than global anyway, such
as for pmwebd inbound & outbound access controls.

Regarding derived metrics on page 5, ditto.  This need not be a global
variable at all; it could be contextual.  Or the only global data
could be the textual definitions, lightly parsed, collected at program
startup or the pm*Derived* calls, and that takes no processing time at
all.  All the real work (PMAPI level validation of the metrics, etc.)
takes place during actual context setup via __dminit etc., and that
does not need to lock the definitions - just work on a snapshot copy.
In a way, it's a good demonstration for how a small local lock could
do the job.


I'm very glad to be having this discussion, Ken.  Let's keep it going.


- FChE

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#14925): https://groups.io/g/pcp/message/14925
View All Messages In Topic (2): https://groups.io/g/pcp/topic/3819767
Mute This Topic: https://groups.io/mt/3819767?uid=174580
New Topic: https://groups.io/g/pcp/post

Change Your Subscription: https://groups.io/g/pcp/editsub?uid=174580

Group Home: https://groups.io/g/pcp
Contact Group Owner: [email protected]

Terms of Service: https://groups.io/static/tos

Unsubscribe: https://groups.io/g/pcp/leave/354243/563757577/xyzzy

-=-=-
pcp mailing list
[email protected]
https://groups.io/g/pcp/messages
-=-=-=-=-=-=-=-=-=-=-=-