public inbox for [email protected]
help / color / mirror / Atom feedRe: Some performance degradation in REL_16 vs REL_15
5+ messages / 5 participants
[nested] [flat]
* Re: Some performance degradation in REL_16 vs REL_15
@ 2023-10-13 02:05 Andres Freund <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Andres Freund @ 2023-10-13 02:05 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; +Cc: pgsql-hackers; David Rowley <[email protected]>; Michael Paquier <[email protected]>
Hi,
On 2023-10-12 11:00:22 +0300, Anton A. Melnikov wrote:
> Found that simple test pgbench -c20 -T20 -j8 gives approximately
> for REL_15_STABLE at 5143f76: 336+-1 TPS
> and
> for REL_16_STABLE at 4ac7635f: 324+-1 TPS
>
> The performance drop is approximately 3,5% while the corrected standard deviation is only 0.3%.
> See the raw_data.txt attached.
Could you provide a bit more details about how you ran the benchmark? The
reason I am asking is that ~330 TPS is pretty slow for -c20. Even on spinning
rust and using the default settings, I get considerably higher results.
Oh - I do get results closer to yours if I use pgbench scale 1, causing a lot
of row level contention. What scale did you use?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Some performance degradation in REL_16 vs REL_15
@ 2023-10-16 08:04 Anton A. Melnikov <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Anton A. Melnikov @ 2023-10-16 08:04 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers; David Rowley <[email protected]>; Michael Paquier <[email protected]>
On 13.10.2023 05:05, Andres Freund wrote:
> Could you provide a bit more details about how you ran the benchmark? The
> reason I am asking is that ~330 TPS is pretty slow for -c20. Even on spinning
> rust and using the default settings, I get considerably higher results.
>
> Oh - I do get results closer to yours if I use pgbench scale 1, causing a lot
> of row level contention. What scale did you use?
I use default scale of 1.
And run the command sequence:
$pgbench -i bench
$sleep 1
$pgbench -c20 -T10 -j8
in a loop to get similar initial conditions for every "pgbench -c20 -T10 -j8" run.
Thanks for your interest!
With the best wishes,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Some performance degradation in REL_16 vs REL_15
@ 2023-10-18 03:45 邱宇航 <[email protected]>
parent: Anton A. Melnikov <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: 邱宇航 @ 2023-10-18 03:45 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers; David Rowley <[email protected]>; Michael Paquier <[email protected]>
I wrote a script and test on branch REL_[10-16]_STABLE, and do see performance drop in REL_13_STABLE, which is about 1~2%.
scale round 10 11 12 13 14 15 16
1 1 7922.2 8018.3 8102.8 7838.3 7829.2 7870.0 7846.1
2 7922.4 7923.5 8090.3 7887.7 7912.4 7815.2 7865.6
3 7937.6 7964.9 8012.8 7918.5 7879.4 7786.4 7981.1
4 8000.4 7959.5 8141.1 7886.3 7840.9 7863.5 8022.4
5 7921.8 7945.5 8005.2 7993.7 7957.0 7803.8 7899.8
6 7893.8 7895.1 8017.2 7879.8 7880.9 7911.4 7909.2
7 7879.3 7853.5 8071.7 7956.2 7876.7 7863.3 7986.3
8 7980.5 7964.1 8119.2 8015.2 7877.6 7784.9 7923.6
9 8083.9 7946.4 7960.3 7913.9 7924.6 7867.7 7928.6
10 7971.2 7991.8 7999.5 7812.4 7824.3 7831.0 7953.4
AVG 7951.3 7946.3 8052.0 7910.2 7880.3 7839.7 7931.6
MED 7930.0 7952.9 8044.5 7900.8 7878.5 7847.1 7926.1
10 1 41221.5 41394.8 40926.8 40566.6 41661.3 40511.9 40961.8
2 40974.0 40697.9 40842.4 40269.2 41127.7 40795.5 40814.9
3 41453.5 41426.4 41066.2 40890.9 41018.6 40897.3 40891.7
4 41691.9 40294.9 41189.8 40873.8 41539.7 40943.2 40643.8
5 40843.4 40855.5 41243.8 40351.3 40863.2 40839.6 40795.5
6 40969.3 40897.9 41380.8 40734.7 41269.3 41301.0 41061.0
7 40981.1 41119.5 41158.0 40834.6 40967.1 40790.6 41061.6
8 41006.4 41205.9 40740.3 40978.7 40742.4 40951.6 41242.1
9 41089.9 41129.7 40648.3 40622.1 40782.0 40460.5 40877.9
10 41280.3 41462.7 41316.4 40728.0 40983.9 40747.0 40964.6
AVG 41151.1 41048.5 41051.3 40685.0 41095.5 40823.8 40931.5
MED 41048.2 41124.6 41112.1 40731.3 41001.3 40817.6 40926.7
100 1 43429.0 43190.2 44099.3 43941.5 43883.3 44215.0 44604.9
2 43281.7 43795.2 44963.6 44331.5 43559.7 43571.5 43403.9
3 43749.0 43614.1 44616.7 43759.5 43617.8 43530.3 43362.4
4 43362.0 43197.3 44296.7 43692.4 42020.5 43607.3 43081.8
5 43373.4 43288.0 44240.9 43795.0 43630.6 43576.7 43512.0
6 43637.0 43385.2 45130.1 43792.5 43635.4 43905.2 43371.2
7 43621.2 43474.2 43735.0 43592.2 43889.7 43947.7 43369.8
8 43351.0 43937.5 44285.6 43877.2 43771.1 43879.1 43680.4
9 43481.3 43700.5 44119.9 43786.9 43440.8 44083.1 43563.2
10 43238.7 43559.5 44310.8 43406.0 44306.6 43376.3 43242.7
AVG 43452.4 43514.2 44379.9 43797.5 43575.6 43769.2 43519.2
MED 43401.2 43516.8 44291.2 43789.7 43633.0 43743.2 43387.5
The script looks like:
initdb data >/dev/null 2>&1 #initdb on every round
pg_ctl -D data -l logfile start >/dev/null 2>&1 #start without changing any setting
pgbench -i postgres $scale >/dev/null 2>&1
sleep 1 >/dev/null 2>&1
pgbench -c20 -T10 -j8
And here is the pg_config output:
...
CONFIGURE = '--enable-debug' '--prefix=/home/postgres/base' '--enable-depend' 'PKG_CONFIG_PATH=/usr/local/lib64/pkgconfig::/usr/lib/pkgconfig'
CC = gcc
CPPFLAGS = -D_GNU_SOURCE
CFLAGS = -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2
CFLAGS_SL = -fPIC
LDFLAGS = -Wl,--as-needed -Wl,-rpath,'/home/postgres/base/lib',--enable-new-dtags
LDFLAGS_EX =
LDFLAGS_SL =
LIBS = -lpgcommon -lpgport -lz -lreadline -lpthread -lrt -ldl -lm
VERSION = PostgreSQL 16.0
—-
Yuhang Qiu
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Some performance degradation in REL_16 vs REL_15
@ 2023-10-18 04:14 Tom Lane <[email protected]>
parent: 邱宇航 <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Tom Lane @ 2023-10-18 04:14 UTC (permalink / raw)
To: 邱宇航 <[email protected]>; +Cc: Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; pgsql-hackers; David Rowley <[email protected]>; Michael Paquier <[email protected]>
=?utf-8?B?6YKx5a6H6Iiq?= <[email protected]> writes:
> I wrote a script and test on branch REL_[10-16]_STABLE, and do see performance drop in REL_13_STABLE, which is about 1~2%.
I'm really skeptical that we should pay much attention to these numbers.
You've made several of the mistakes that we typically tell people not to
make when using pgbench:
* scale <= number of sessions means you're measuring a lot of
row-update contention
* once you crank up the scale enough to avoid that problem, running
with the default shared_buffers seems like a pretty poor choice
* 10-second runtime is probably an order of magnitude too small
to get useful, reliable numbers
On top of all that, discrepancies on the order of a percent or two
commonly arise from hard-to-control-for effects like the cache
alignment of hot spots in different parts of the code. That means
that you can see changes of that size from nothing more than
day-to-day changes in completely unrelated parts of the code.
I'd get excited about say a 10% performance drop, because that's
probably more than noise; but I'm not convinced that any of the
differences you show here are more than noise.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v18 2/8] Row pattern recognition patch (parse/analysis).
@ 2024-05-11 07:11 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Tatsuo Ishii @ 2024-05-11 07:11 UTC (permalink / raw)
---
src/backend/parser/parse_agg.c | 7 +
src/backend/parser/parse_clause.c | 296 +++++++++++++++++++++++++++++-
src/backend/parser/parse_expr.c | 4 +
src/backend/parser/parse_func.c | 3 +
4 files changed, 309 insertions(+), 1 deletion(-)
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index bee7d8346a..9bc22a836a 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -577,6 +577,10 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
+
/*
* There is intentionally no default: case here, so that the
* compiler will warn if we add a new ParseExprKind without
@@ -967,6 +971,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
case EXPR_KIND_CYCLE_MARK:
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
/*
* There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 8118036495..9762dce81f 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -98,7 +98,14 @@ static WindowClause *findWindowClause(List *wclist, const char *name);
static Node *transformFrameOffset(ParseState *pstate, int frameOptions,
Oid rangeopfamily, Oid rangeopcintype, Oid *inRangeFunc,
Node *clause);
-
+static void transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist);
+static List *transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist);
+static void transformPatternClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef);
+static List *transformMeasureClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef);
/*
* transformFromClause -
@@ -2956,6 +2963,10 @@ transformWindowDefinitions(ParseState *pstate,
rangeopfamily, rangeopcintype,
&wc->endInRangeFunc,
windef->endOffset);
+
+ /* Process Row Pattern Recognition related clauses */
+ transformRPR(pstate, wc, windef, targetlist);
+
wc->winref = winref;
result = lappend(result, wc);
@@ -3820,3 +3831,286 @@ transformFrameOffset(ParseState *pstate, int frameOptions,
return node;
}
+
+/*
+ * transformRPR
+ * Process Row Pattern Recognition related clauses
+ */
+static void
+transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist)
+{
+ /*
+ * Window definition exists?
+ */
+ if (windef == NULL)
+ return;
+
+ /*
+ * Row Pattern Common Syntax clause exists?
+ */
+ if (windef->rpCommonSyntax == NULL)
+ return;
+
+ /* Check Frame option. Frame must start at current row */
+ if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("FRAME must start at current row when row patttern recognition is used")));
+
+ /* Transform AFTER MACH SKIP TO clause */
+ wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo;
+
+ /* Transform AFTER MACH SKIP TO variable */
+ wc->rpSkipVariable = windef->rpCommonSyntax->rpSkipVariable;
+
+ /* Transform SEEK or INITIAL clause */
+ wc->initial = windef->rpCommonSyntax->initial;
+
+ /* Transform DEFINE clause into list of TargetEntry's */
+ wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist);
+
+ /* Check PATTERN clause and copy to patternClause */
+ transformPatternClause(pstate, wc, windef);
+
+ /* Transform MEASURE clause */
+ transformMeasureClause(pstate, wc, windef);
+}
+
+/*
+ * transformDefineClause Process DEFINE clause and transform ResTarget into
+ * list of TargetEntry.
+ *
+ * XXX we only support column reference in row pattern definition search
+ * condition, e.g. "price". <row pattern definition variable name>.<column
+ * reference> is not supported, e.g. "A.price".
+ */
+static List *
+transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist)
+{
+ /* DEFINE variable name initials */
+ static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz";
+
+ ListCell *lc,
+ *l;
+ ResTarget *restarget,
+ *r;
+ List *restargets;
+ List *defineClause;
+ char *name;
+ int initialLen;
+ int i;
+
+ /*
+ * If Row Definition Common Syntax exists, DEFINE clause must exist. (the
+ * raw parser should have already checked it.)
+ */
+ Assert(windef->rpCommonSyntax->rpDefs != NULL);
+
+ /*
+ * Check and add "A AS A IS TRUE" if pattern variable is missing in DEFINE
+ * per the SQL standard.
+ */
+ restargets = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpPatterns)
+ {
+ A_Expr *a;
+ bool found = false;
+
+ if (!IsA(lfirst(lc), A_Expr))
+ ereport(ERROR,
+ errmsg("node type is not A_Expr"));
+
+ a = (A_Expr *) lfirst(lc);
+ name = strVal(a->lexpr);
+
+ foreach(l, windef->rpCommonSyntax->rpDefs)
+ {
+ restarget = (ResTarget *) lfirst(l);
+
+ if (!strcmp(restarget->name, name))
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ /*
+ * "name" is missing. So create "name AS name IS TRUE" ResTarget
+ * node and add it to the temporary list.
+ */
+ A_Const *n;
+
+ restarget = makeNode(ResTarget);
+ n = makeNode(A_Const);
+ n->val.boolval.type = T_Boolean;
+ n->val.boolval.boolval = true;
+ n->location = -1;
+ restarget->name = pstrdup(name);
+ restarget->indirection = NIL;
+ restarget->val = (Node *) n;
+ restarget->location = -1;
+ restargets = lappend((List *) restargets, restarget);
+ }
+ }
+
+ if (list_length(restargets) >= 1)
+ {
+ /* add missing DEFINEs */
+ windef->rpCommonSyntax->rpDefs =
+ list_concat(windef->rpCommonSyntax->rpDefs, restargets);
+ list_free(restargets);
+ }
+
+ /*
+ * Check for duplicate row pattern definition variables. The standard
+ * requires that no two row pattern definition variable names shall be
+ * equivalent.
+ */
+ restargets = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpDefs)
+ {
+ restarget = (ResTarget *) lfirst(lc);
+ name = restarget->name;
+
+ /*
+ * Add DEFINE expression (Restarget->val) to the targetlist as a
+ * TargetEntry if it does not exist yet. Planner will add the column
+ * ref var node to the outer plan's target list later on. This makes
+ * DEFINE expression could access the outer tuple while evaluating
+ * PATTERN.
+ *
+ * XXX: adding whole expressions of DEFINE to the plan.targetlist is
+ * not so good, because it's not necessary to evalute the expression
+ * in the target list while running the plan. We should extract the
+ * var nodes only then add them to the plan.targetlist.
+ */
+ findTargetlistEntrySQL99(pstate, (Node *) restarget->val,
+ targetlist, EXPR_KIND_RPR_DEFINE);
+
+ /*
+ * Make sure that the row pattern definition search condition is a
+ * boolean expression.
+ */
+ transformWhereClause(pstate, restarget->val,
+ EXPR_KIND_RPR_DEFINE, "DEFINE");
+
+ foreach(l, restargets)
+ {
+ char *n;
+
+ r = (ResTarget *) lfirst(l);
+ n = r->name;
+
+ if (!strcmp(n, name))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("row pattern definition variable name \"%s\" appears more than once in DEFINE clause",
+ name),
+ parser_errposition(pstate, exprLocation((Node *) r))));
+ }
+ restargets = lappend(restargets, restarget);
+ }
+ list_free(restargets);
+
+ /*
+ * Create list of row pattern DEFINE variable name's initial. We assign
+ * [a-z] to them (up to 26 variable names are allowed).
+ */
+ restargets = NIL;
+ i = 0;
+ initialLen = strlen(defineVariableInitials);
+
+ foreach(lc, windef->rpCommonSyntax->rpDefs)
+ {
+ char initial[2];
+
+ restarget = (ResTarget *) lfirst(lc);
+ name = restarget->name;
+
+ if (i >= initialLen)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("number of row pattern definition variable names exceeds %d",
+ initialLen),
+ parser_errposition(pstate,
+ exprLocation((Node *) restarget))));
+ }
+ initial[0] = defineVariableInitials[i++];
+ initial[1] = '\0';
+ wc->defineInitial = lappend(wc->defineInitial,
+ makeString(pstrdup(initial)));
+ }
+
+ defineClause = transformTargetList(pstate, windef->rpCommonSyntax->rpDefs,
+ EXPR_KIND_RPR_DEFINE);
+
+ /* mark column origins */
+ markTargetListOrigins(pstate, defineClause);
+
+ /* mark all nodes in the DEFINE clause tree with collation information */
+ assign_expr_collations(pstate, (Node *) defineClause);
+
+ return defineClause;
+}
+
+/*
+ * transformPatternClause
+ * Process PATTERN clause and return PATTERN clause in the raw parse tree
+ */
+static void
+transformPatternClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef)
+{
+ ListCell *lc;
+
+ /*
+ * Row Pattern Common Syntax clause exists?
+ */
+ if (windef->rpCommonSyntax == NULL)
+ return;
+
+ wc->patternVariable = NIL;
+ wc->patternRegexp = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpPatterns)
+ {
+ A_Expr *a;
+ char *name;
+ char *regexp;
+
+ if (!IsA(lfirst(lc), A_Expr))
+ ereport(ERROR,
+ errmsg("node type is not A_Expr"));
+
+ a = (A_Expr *) lfirst(lc);
+ name = strVal(a->lexpr);
+
+ wc->patternVariable = lappend(wc->patternVariable, makeString(pstrdup(name)));
+ regexp = strVal(lfirst(list_head(a->name)));
+
+ wc->patternRegexp = lappend(wc->patternRegexp, makeString(pstrdup(regexp)));
+ }
+}
+
+/*
+ * transformMeasureClause
+ * Process MEASURE clause
+ * XXX MEASURE clause is not supported yet
+ */
+static List *
+transformMeasureClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef)
+{
+ if (windef->rowPatternMeasures == NIL)
+ return NIL;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("%s", "MEASURE clause is not supported yet"),
+ parser_errposition(pstate, exprLocation((Node *) windef->rowPatternMeasures))));
+ return NIL;
+}
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index aba3546ed1..eb138087bf 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -578,6 +578,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
case EXPR_KIND_COPY_WHERE:
case EXPR_KIND_GENERATED_COLUMN:
case EXPR_KIND_CYCLE_MARK:
+ case EXPR_KIND_RPR_DEFINE:
/* okay */
break;
@@ -1817,6 +1818,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
case EXPR_KIND_VALUES:
case EXPR_KIND_VALUES_SINGLE:
case EXPR_KIND_CYCLE_MARK:
+ case EXPR_KIND_RPR_DEFINE:
/* okay */
break;
case EXPR_KIND_CHECK_CONSTRAINT:
@@ -3197,6 +3199,8 @@ ParseExprKindName(ParseExprKind exprKind)
return "GENERATED AS";
case EXPR_KIND_CYCLE_MARK:
return "CYCLE";
+ case EXPR_KIND_RPR_DEFINE:
+ return "DEFINE";
/*
* There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 9b23344a3b..4c482abb30 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2658,6 +2658,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
case EXPR_KIND_CYCLE_MARK:
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
/*
* There is intentionally no default: case here, so that the
--
2.25.1
----Next_Part(Sat_May_11_16_23_07_2024_789)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v18-0003-Row-pattern-recognition-patch-rewriter.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-05-11 07:11 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-10-13 02:05 Re: Some performance degradation in REL_16 vs REL_15 Andres Freund <[email protected]>
2023-10-16 08:04 ` Anton A. Melnikov <[email protected]>
2023-10-18 03:45 ` 邱宇航 <[email protected]>
2023-10-18 04:14 ` Tom Lane <[email protected]>
2024-05-11 07:11 [PATCH v18 2/8] Row pattern recognition patch (parse/analysis). Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox