On 07/22/2015 09:10 AM, Kyotaro HORIGUCHI wrote:
Hello,

At Tue, 21 Jul 2015 14:28:25 +0300, Ildus Kurbangaliev 
<i.kurbangal...@postgrespro.ru> wrote in <55ae2cd9.4050...@postgrespro.ru>
On 07/21/2015 01:18 PM, Andres Freund wrote:
On 2015-07-21 13:11:36 +0300, Ildus Kurbangaliev wrote:
     /*
    * Top-level transactions are identified by VirtualTransactionIDs
    * comprising
diff --git a/src/include/storage/lwlock.h
b/src/include/storage/lwlock.h
index cff3b99..55b0687 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -58,6 +58,9 @@ typedef struct LWLock
   #ifdef LOCK_DEBUG
        struct PGPROC *owner;           /* last exlusive owner of the lock */
   #endif
+
+ /* LWLock group, initialized as -1, calculated in first acquire */
+       int group;
   } LWLock;
I'd very much like to avoid increasing the size of struct LWLock. We
have a lot of those and I'd still like to inline them with the buffer
descriptors. Why do we need a separate group and can't reuse the
tranche?  That might require creating a few more tranches, but ...?

Andres
Do you mean moving LWLocks defined by offsets and with dynamic sizes
to separate tranches?
I think it is too much for the purpose. Only two new tranches and
maybe one or some new members (maybe representing the group) of
trances will do, I suppose.

Can you explain why only two new tranches?
There is 13 types of lwlocks (besides individual), and we need separate them somehow.
It sounds like an good option, but it will require refactoring of
current tranches. In current implementation
i tried not change current things.
Now one of the most controversial points of this patch is the
details of the implement, which largely affects performance drag,
maybe.


 From the viewpoint of performance, I have some comments on the feature:

  - LWLockReportStat runs a linear search loop which I suppose
    should be avoided even if the loop count is rather small for
    LWLocks, as Fujii-san said upthread or anywhere.
It runs only one time in first acquirement. In previous patch
it was much heavier. Anyway this code will be removed if we
split main tranche to smaller tranches.
  - Currently pg_stat_activity view is the only API, which would
    be a bit heavy for sampling use. It'd be appreciated to have a
    far lighter means to know the same informtion.
Yes, pg_stat_activity is just information about current wait,
and it's too heavy for sampling.  Main goal of this patch was
creating base structures that can be used later.

--
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to