Improvement of search for a binary operator - Mailing list pgsql-patches

From Atsushi Ogawa
Subject Improvement of search for a binary operator
Date
Msg-id 4450B8CA.8070704@hi-ho.ne.jp
Whole thread Raw
Responses Re: Improvement of search for a binary operator  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Improvement of search for a binary operator  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
Here is a gprof result of the pgbench.

  %   cumulative   self             self    total
 time   seconds   seconds    calls  s/call  s/call  name
  4.77      4.04     4.04    14000    0.00    0.00  OpernameGetCandidates
  4.70      8.02     3.98  2007008    0.00    0.00
HeapTupleSatisfiesSnapshot
  4.50     11.83     3.81    14001    0.00    0.00  base_yyparse
  3.78     15.03     3.20  2838312    0.00    0.00  AllocSetAlloc
  3.73     18.18     3.15    40001    0.00    0.00  heapgetpage

The OpernameGetCandidates called from oper. The function of oper is
search for a binary operator. It does the following processing:

(1)Create candidates of operator that matches operator name and
operator kind by OpernameGetCandidates.
(2)Find an operator that matches exactly ltypeId and rtypeId from
the candidates of operator by binary_oper_exact.
(3)If not found, find an operator from the candidates of operator by
oper_select_candidate. The oper_select_candidate accepts coerce type
and resolve the conflict.

I think that we can search the system catalog cache instead of
retrieval from the candidates of operator in the binary_oper_exact,
and reverse the order of executing (1) and (2) for performance
improvement. Because for a general operator such as '=', the number
of the candidates of operator exceeds 50. And in common cases the
typeIds matches exactly.

I tested following commands:

pgbench -i
pgbench -t 4000 (5 times)

results (tps):
             1       2       3       4       5     average
-----------------------------------------------------------
 original: 214.33  184.60  192.52  158.62  170.04  184.02
 patched : 223.86  198.72  207.48  165.70  179.67  195.09

regards,

---
Atsushi Ogawa

*** ./src/backend/catalog/namespace.c.orig    Tue Apr 25 23:16:57 2006
--- ./src/backend/catalog/namespace.c    Wed Apr 26 00:14:33 2006
***************
*** 681,686 ****
--- 681,744 ----
      return visible;
  }

+ /*
+  * OpernameGet
+  *        Given a possibly-qualified operator name, left and right type IDs,
+  *        find an operator in the current search path.
+  */
+ Oid
+ OpernameGet(List *names, Oid ltypeId, Oid rtypeId)
+ {
+     char       *schemaname;
+     char       *opername;
+     Oid            namespaceId;
+     HeapTuple    tup = NULL;
+
+     /* deconstruct the name list */
+     DeconstructQualifiedName(names, &schemaname, &opername);
+
+     if (schemaname)
+     {
+         /* use exact schema given */
+         namespaceId = LookupExplicitNamespace(schemaname);
+
+         tup = SearchSysCache(OPERNAMENSP,
+                              CStringGetDatum(opername),
+                              ObjectIdGetDatum(ltypeId),
+                              ObjectIdGetDatum(rtypeId),
+                              ObjectIdGetDatum(namespaceId));
+     }
+     else
+     {
+         /* we need namespace search */
+         ListCell   *nsp;
+
+         recomputeNamespacePath();
+
+         foreach(nsp, namespaceSearchPath)
+         {
+             namespaceId = lfirst_oid(nsp);
+
+             tup = SearchSysCache(OPERNAMENSP,
+                                  CStringGetDatum(opername),
+                                  ObjectIdGetDatum(ltypeId),
+                                  ObjectIdGetDatum(rtypeId),
+                                  ObjectIdGetDatum(namespaceId));
+             if (HeapTupleIsValid(tup))
+                 break;
+         }
+     }
+
+     if (HeapTupleIsValid(tup))
+     {
+         Oid operOid = HeapTupleGetOid(tup);
+         ReleaseSysCache(tup);
+         return operOid;
+     }
+
+     return InvalidOid;
+ }
+

  /*
   * OpernameGetCandidates
*** ./src/backend/parser/parse_oper.c.orig    Tue Apr 25 22:12:52 2006
--- ./src/backend/parser/parse_oper.c    Wed Apr 26 00:19:04 2006
***************
*** 29,36 ****
  #include "utils/typcache.h"


! static Oid binary_oper_exact(Oid arg1, Oid arg2,
!                   FuncCandidateList candidates);
  static FuncDetailCode oper_select_candidate(int nargs,
                        Oid *input_typeids,
                        FuncCandidateList candidates,
--- 29,35 ----
  #include "utils/typcache.h"


! static Oid binary_oper_exact(List *opname, Oid arg1, Oid arg2);
  static FuncDetailCode oper_select_candidate(int nargs,
                        Oid *input_typeids,
                        FuncCandidateList candidates,
***************
*** 387,397 ****
   * be reduced to its base type to find an "exact" match.
   */
  static Oid
! binary_oper_exact(Oid arg1, Oid arg2,
!                   FuncCandidateList candidates)
  {
      FuncCandidateList cand;
      bool        was_unknown = false;

      /* Unspecified type for one of the arguments? then use the other */
      if ((arg1 == UNKNOWNOID) && (arg2 != InvalidOid))
--- 386,396 ----
   * be reduced to its base type to find an "exact" match.
   */
  static Oid
! binary_oper_exact(List *opname, Oid arg1, Oid arg2)
  {
      FuncCandidateList cand;
      bool        was_unknown = false;
+     Oid            operOid;

      /* Unspecified type for one of the arguments? then use the other */
      if ((arg1 == UNKNOWNOID) && (arg2 != InvalidOid))
***************
*** 405,415 ****
          was_unknown = true;
      }

!     for (cand = candidates; cand != NULL; cand = cand->next)
!     {
!         if (arg1 == cand->args[0] && arg2 == cand->args[1])
!             return cand->oid;
!     }

      if (was_unknown)
      {
--- 404,412 ----
          was_unknown = true;
      }

!     operOid = OpernameGet(opname, arg1, arg2);
!     if (OidIsValid(operOid))
!         return operOid;

      if (was_unknown)
      {
***************
*** 418,428 ****

          if (basetype != arg1)
          {
!             for (cand = candidates; cand != NULL; cand = cand->next)
!             {
!                 if (basetype == cand->args[0] && basetype == cand->args[1])
!                     return cand->oid;
!             }
          }
      }

--- 415,423 ----

          if (basetype != arg1)
          {
!             operOid = OpernameGet(opname, basetype, basetype);
!             if (OidIsValid(operOid))
!                 return operOid;
          }
      }

***************
*** 509,530 ****
      FuncDetailCode fdresult = FUNCDETAIL_NOTFOUND;
      HeapTuple    tup = NULL;

!     /* Get binary operators of given name */
!     clist = OpernameGetCandidates(opname, 'b');

!     /* No operators found? Then fail... */
!     if (clist != NULL)
      {
          /*
!          * Check for an "exact" match.
           */
-         operOid = binary_oper_exact(ltypeId, rtypeId, clist);
-         if (!OidIsValid(operOid))
-         {
-             /*
-              * Otherwise, search for the most suitable candidate.
-              */

              /*
               * Unspecified type for one of the arguments? then use the other
               * (XXX this is probably dead code?)
--- 504,526 ----
      FuncDetailCode fdresult = FUNCDETAIL_NOTFOUND;
      HeapTuple    tup = NULL;

!     /*
!      * Check for an "exact" match.
!      */
!     operOid = binary_oper_exact(opname, ltypeId, rtypeId);

!     if (!OidIsValid(operOid))
      {
          /*
!          * Otherwise, search for the most suitable candidate.
           */

+         /* Get binary operators of given name */
+         clist = OpernameGetCandidates(opname, 'b');
+
+         /* No operators found? Then fail... */
+         if (clist != NULL)
+         {
              /*
               * Unspecified type for one of the arguments? then use the other
               * (XXX this is probably dead code?)
***************
*** 537,548 ****
              inputOids[1] = rtypeId;
              fdresult = oper_select_candidate(2, inputOids, clist, &operOid);
          }
-         if (OidIsValid(operOid))
-             tup = SearchSysCache(OPEROID,
-                                  ObjectIdGetDatum(operOid),
-                                  0, 0, 0);
      }

      if (!HeapTupleIsValid(tup) && !noError)
          op_error(pstate, opname, 'b', ltypeId, rtypeId, fdresult, location);

--- 533,545 ----
              inputOids[1] = rtypeId;
              fdresult = oper_select_candidate(2, inputOids, clist, &operOid);
          }
      }

+     if (OidIsValid(operOid))
+         tup = SearchSysCache(OPEROID,
+                              ObjectIdGetDatum(operOid),
+                              0, 0, 0);
+
      if (!HeapTupleIsValid(tup) && !noError)
          op_error(pstate, opname, 'b', ltypeId, rtypeId, fdresult, location);


pgsql-patches by date:

Previous
From: "Magnus Hagander"
Date:
Subject: Re: pgstat: delayed write of stats file
Next
From: Bruce Momjian
Date:
Subject: Re: be-secure.c patch