Re: pgbench - rework variable management

2020-09-19 Thread Fabien COELHO 


Bonjour Michaël,


https://www.postgresql.org/message-id/CAMN686ExUKturcWp4POaaVz3gR3hauSGBjOCd0E-Jh1zEXqf_Q%40mail.gmail.com


Since then, the patch is failing to apply.  As this got zero activity
for the last six months, I am marking the entry as returned with
feedback in the CF app.


Hmmm… I did not notice it did not apply anymore. I do not have much time 
to contribute much this round and probably the next as well, so fine with 
me.


--
Fabien.

Re: pgbench - rework variable management

2020-09-16 Thread Michael Paquier 
On Fri, Mar 27, 2020 at 06:42:49PM -0400, David Steele wrote:
> Regarding Windows testing you may find this thread useful:
> 
> https://www.postgresql.org/message-id/CAMN686ExUKturcWp4POaaVz3gR3hauSGBjOCd0E-Jh1zEXqf_Q%40mail.gmail.com

Since then, the patch is failing to apply.  As this got zero activity
for the last six months, I am marking the entry as returned with
feedback in the CF app.
--
Michael


signature.asc
Description: PGP signature

Re: pgbench - rework variable management

2020-03-27 Thread David Steele 

On 3/27/20 6:25 PM, Fabien COELHO wrote:



Patch v5 is a rebase with some adjustements.


This patch is failing on the Windows build:
https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.85698 



I'm not sure if this had been fixed in v3 and this is a new issue or 
if it has been failing all along.  Either way, it should be updated.


I don't do windows, so the mutex stuff for windows is just blind 
programming.



Marked Waiting on Author.

BTW -- sorry if I seem to be picking on your patches but these happen 
to be the patches with the longest time since any activity.


Basically, my areas of interest do not match committers' areas of interest.

v6 is a yet-again blind attempt at fixing the windows mutex. If someone 
with a windows could tell me if it is ok, and if not what to fix, it 
would be great.


Regarding Windows testing you may find this thread useful:

https://www.postgresql.org/message-id/CAMN686ExUKturcWp4POaaVz3gR3hauSGBjOCd0E-Jh1zEXqf_Q%40mail.gmail.com

--
-David
da...@pgmasters.net

Re: pgbench - rework variable management

2020-03-27 Thread Fabien COELHO 



Patch v5 is a rebase with some adjustements.


This patch is failing on the Windows build:
https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.85698

I'm not sure if this had been fixed in v3 and this is a new issue or if it 
has been failing all along.  Either way, it should be updated.


I don't do windows, so the mutex stuff for windows is just blind 
programming.



Marked Waiting on Author.

BTW -- sorry if I seem to be picking on your patches but these happen to be 
the patches with the longest time since any activity.


Basically, my areas of interest do not match committers' areas of 
interest.


v6 is a yet-again blind attempt at fixing the windows mutex. If someone 
with a windows could tell me if it is ok, and if not what to fix, it would 
be great.


--
Fabien.diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index f402fe7b91..e1d3ef9bb3 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -10,6 +10,7 @@ include $(top_builddir)/src/Makefile.global
 OBJS = \
 	$(WIN32RES) \
 	exprparse.o \
+	symbol_table.o \
 	pgbench.o
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 85d61caa9f..ca21ee012e 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -211,7 +211,7 @@ make_variable(char *varname)
 	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
 
 	expr->etype = ENODE_VARIABLE;
-	expr->u.variable.varname = varname;
+	expr->u.variable.varnum = getOrCreateSymbolId(symbol_table, varname);
 	return expr;
 }
 
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 430bff38a6..a3c5ea348d 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -207,19 +207,19 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 {digit}+		{
 	if (!strtoint64(yytext, true, >ival))
 		expr_yyerror_more(yyscanner, "bigint constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return INTEGER_CONST;
 }
 {digit}+(\.{digit}*)?([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 \.{digit}+([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 {alpha}{alnum}*	{
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index b8864c6ae5..3e4b0d7712 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -251,24 +251,32 @@ const char *progname;
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /*
- * Variable definitions.
+ * Variable contents.
+ *
+ * Variable names are managed in symbol_table with a number.
  *
  * If a variable only has a string value, "svalue" is that value, and value is
- * "not set".  If the value is known, "value" contains the value (in any
- * variant).
+ * not set.
+ *
+ * If the value is known, "value" contains the value (in any variant).
  *
  * In this case "svalue" contains the string equivalent of the value, if we've
- * had occasion to compute that, or NULL if we haven't.
+ * had occasion to compute that, or an empty string if we haven't.
+ *
+ * The string length is arbitrary limited to benefit from static allocation.
  */
+#define SVALUE_SIZE	128
 typedef struct
 {
-	char	   *name;			/* variable's name */
-	char	   *svalue;			/* its value in string form, if known */
-	PgBenchValue value;			/* actual variable's value */
+	int			number;/* variable symbol number, -1 if unset */
+	char			svalue[SVALUE_SIZE];	/* value in string form, if known */
+	PgBenchValue	value;	/* actual value, if known */
 } Variable;
 
+#define undefined_variable_value(v)		\
+	v.svalue[0] == '\0' && v.value.type == PGBT_NO_VALUE
+
 #define MAX_SCRIPTS		128		/* max number of SQL scripts allowed */
-#define SHELL_COMMAND_SIZE	256 /* maximum size allowed for shell command */
 
 /*
  * Simple data structure to keep stats about something.
@@ -414,7 +422,6 @@ typedef struct
 	/* client variables */
 	Variable   *variables;		/* array of variable definitions */
 	int			nvariables;		/* number of variables */
-	bool		vars_sorted;	/* are variables sorted by name? */
 
 	/* various times about current transaction */
 	int64		txn_scheduled;	/* scheduled start time of transaction (usec) */
@@ -427,6 +434,9 @@ typedef struct
 	/* per client collected stats */
 	int64		cnt;			/* client transaction count, for -t */
 	int			ecnt;			/* error count */
+
+	/* next buffer should be at thread level, but it would change functions... */
+	PQExpBufferData	buf;		/* persistant buffer to avoid malloc/free cycles */
 } CState;
 
 /*
@@ -512,6 +522,7 @@ static const char *QUERYMODE[] = {"simple",

Re: pgbench - rework variable management

2020-03-27 Thread David Steele 

Hi Fabien,

On 1/9/20 5:04 PM, Fabien COELHO wrote:



Patch v4 is a just a rebase.


Patch v5 is a rebase with some adjustements.


This patch is failing on the Windows build:
https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.85698

I'm not sure if this had been fixed in v3 and this is a new issue or if 
it has been failing all along.  Either way, it should be updated.


Marked Waiting on Author.

BTW -- sorry if I seem to be picking on your patches but these happen to 
be the patches with the longest time since any activity.


Regards,
--
-David
da...@pgmasters.net

Re: pgbench - rework variable management

2020-01-09 Thread Fabien COELHO 



Patch v4 is a just a rebase.


Patch v5 is a rebase with some adjustements.

--
Fabien.diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index f402fe7b91..e1d3ef9bb3 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -10,6 +10,7 @@ include $(top_builddir)/src/Makefile.global
 OBJS = \
 	$(WIN32RES) \
 	exprparse.o \
+	symbol_table.o \
 	pgbench.o
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 85d61caa9f..ca21ee012e 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -211,7 +211,7 @@ make_variable(char *varname)
 	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
 
 	expr->etype = ENODE_VARIABLE;
-	expr->u.variable.varname = varname;
+	expr->u.variable.varnum = getOrCreateSymbolId(symbol_table, varname);
 	return expr;
 }
 
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 430bff38a6..a3c5ea348d 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -207,19 +207,19 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 {digit}+		{
 	if (!strtoint64(yytext, true, >ival))
 		expr_yyerror_more(yyscanner, "bigint constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return INTEGER_CONST;
 }
 {digit}+(\.{digit}*)?([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 \.{digit}+([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 {alpha}{alnum}*	{
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ee1134aea2..22ca43738a 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -251,24 +251,32 @@ const char *progname;
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /*
- * Variable definitions.
+ * Variable contents.
+ *
+ * Variable names are managed in symbol_table with a number.
  *
  * If a variable only has a string value, "svalue" is that value, and value is
- * "not set".  If the value is known, "value" contains the value (in any
- * variant).
+ * not set.
+ *
+ * If the value is known, "value" contains the value (in any variant).
  *
  * In this case "svalue" contains the string equivalent of the value, if we've
- * had occasion to compute that, or NULL if we haven't.
+ * had occasion to compute that, or an empty string if we haven't.
+ *
+ * The string length is arbitrary limited to benefit from static allocation.
  */
+#define SVALUE_SIZE	128
 typedef struct
 {
-	char	   *name;			/* variable's name */
-	char	   *svalue;			/* its value in string form, if known */
-	PgBenchValue value;			/* actual variable's value */
+	int			number;/* variable symbol number, -1 if unset */
+	char			svalue[SVALUE_SIZE];	/* value in string form, if known */
+	PgBenchValue	value;	/* actual value, if known */
 } Variable;
 
+#define undefined_variable_value(v)		\
+	v.svalue[0] == '\0' && v.value.type == PGBT_NO_VALUE
+
 #define MAX_SCRIPTS		128		/* max number of SQL scripts allowed */
-#define SHELL_COMMAND_SIZE	256 /* maximum size allowed for shell command */
 
 /*
  * Simple data structure to keep stats about something.
@@ -414,7 +422,6 @@ typedef struct
 	/* client variables */
 	Variable   *variables;		/* array of variable definitions */
 	int			nvariables;		/* number of variables */
-	bool		vars_sorted;	/* are variables sorted by name? */
 
 	/* various times about current transaction */
 	int64		txn_scheduled;	/* scheduled start time of transaction (usec) */
@@ -427,6 +434,9 @@ typedef struct
 	/* per client collected stats */
 	int64		cnt;			/* client transaction count, for -t */
 	int			ecnt;			/* error count */
+
+	/* next buffer should be at thread level, but it would change functions... */
+	PQExpBufferData	buf;		/* persistant buffer to avoid malloc/free cycles */
 } CState;
 
 /*
@@ -512,6 +522,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
  * varprefix 	SQL commands terminated with \gset have this set
  *to a non NULL value.  If nonempty, it's used to prefix the
  *variable name that receives the value.
+ * set_varnum	variable symbol number for \set and \setshell.
  * expr			Parsed expression, if needed.
  * stats		Time spent in this command.
  */
@@ -524,6 +535,7 @@ typedef struct Command
 	int			argc;
 	char	   *argv[MAX_ARGS];
 	char	   *varprefix;
+	int			set_varnum;
 	PgBenchExpr *expr;
 	SimpleStats stats;
 } Command;
@@ -540,6 +552,9 @@ static ParsedScript sql_script[MAX_SCRIPTS];	/* SQL script files */
 static int	num_scripts;		/* number of scripts in sql_script[] */
 static int64 total_weight = 0;
 
+#define

Re: pgbench - rework variable management

2019-11-06 Thread Fabien COELHO 


Patch v4 is a just a rebase.

--
Fabien.diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index f402fe7b91..e1d3ef9bb3 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -10,6 +10,7 @@ include $(top_builddir)/src/Makefile.global
 OBJS = \
 	$(WIN32RES) \
 	exprparse.o \
+	symbol_table.o \
 	pgbench.o
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 17c9ec32c5..8c4ad7eff3 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -211,7 +211,7 @@ make_variable(char *varname)
 	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
 
 	expr->etype = ENODE_VARIABLE;
-	expr->u.variable.varname = varname;
+	expr->u.variable.varnum = getOrCreateSymbolId(symbol_table, varname);
 	return expr;
 }
 
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index e9020ad231..1c80e5cac6 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -207,19 +207,19 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 {digit}+		{
 	if (!strtoint64(yytext, true, >ival))
 		expr_yyerror_more(yyscanner, "bigint constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return INTEGER_CONST;
 }
 {digit}+(\.{digit}*)?([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 \.{digit}+([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 {alpha}{alnum}*	{
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 14dbc4510c..8bccd332d8 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -251,24 +251,30 @@ const char *progname;
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /*
- * Variable definitions.
+ * Variable contents.
+ *
+ * Variable names are managed in symbol_table with a number.
  *
  * If a variable only has a string value, "svalue" is that value, and value is
- * "not set".  If the value is known, "value" contains the value (in any
- * variant).
+ * "not set".
+ *
+ * If the value is known, "value" contains the value (in any variant).
  *
  * In this case "svalue" contains the string equivalent of the value, if we've
- * had occasion to compute that, or NULL if we haven't.
+ * had occasion to compute that, or an empty string if we haven't.
  */
+#define SVALUE_SIZE	64
 typedef struct
 {
-	char	   *name;			/* variable's name */
-	char	   *svalue;			/* its value in string form, if known */
-	PgBenchValue value;			/* actual variable's value */
+	int			number;/* variable symbol number, -1 if unset */
+	char			svalue[SVALUE_SIZE];	/* value in string form, if known */
+	PgBenchValue	value;	/* actual value, if known */
 } Variable;
 
+#define undefined_variable_value(v)		\
+	v.svalue[0] == '\0' && v.value.type == PGBT_NO_VALUE
+
 #define MAX_SCRIPTS		128		/* max number of SQL scripts allowed */
-#define SHELL_COMMAND_SIZE	256 /* maximum size allowed for shell command */
 
 /*
  * Simple data structure to keep stats about something.
@@ -414,7 +420,6 @@ typedef struct
 	/* client variables */
 	Variable   *variables;		/* array of variable definitions */
 	int			nvariables;		/* number of variables */
-	bool		vars_sorted;	/* are variables sorted by name? */
 
 	/* various times about current transaction */
 	int64		txn_scheduled;	/* scheduled start time of transaction (usec) */
@@ -427,6 +432,9 @@ typedef struct
 	/* per client collected stats */
 	int64		cnt;			/* client transaction count, for -t */
 	int			ecnt;			/* error count */
+
+	/* next buffer should be at thread level, but it would change functions... */
+	PQExpBufferData	buf;		/* persistant buffer to avoid malloc/free cycles */
 } CState;
 
 /*
@@ -512,6 +520,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
  * varprefix 	SQL commands terminated with \gset have this set
  *to a non NULL value.  If nonempty, it's used to prefix the
  *variable name that receives the value.
+ * set_varnum	variable symbol number for \set and \setshell.
  * expr			Parsed expression, if needed.
  * stats		Time spent in this command.
  */
@@ -524,6 +533,7 @@ typedef struct Command
 	int			argc;
 	char	   *argv[MAX_ARGS];
 	char	   *varprefix;
+	int			set_varnum;
 	PgBenchExpr *expr;
 	SimpleStats stats;
 } Command;
@@ -542,6 +552,9 @@ static int64 total_weight = 0;
 
 static int	debug = 0;			/* debug flag */
 
+#define MAX_VARIABLES (32 * MAX_SCRIPTS)
+SymbolTable symbol_table = NULL;	/* also used by exprparse.y */
+
 /* Builtin test scripts */
 typedef struct BuiltinScript
 {
@@ -687,7 +700,7 @@ usage(void)
 		   progname, progname);

Re: pgbench - rework variable management

2019-09-03 Thread Fabien COELHO 


Hello Thomas,


I noticed, but I do not have any windows host so I cannot test locally.

The issue is how to do a mutex on Windows, which does not have pthread so
it has to be emulated. I'll try again by sending a blind update to the
patch and see how it goes.


If you have the patience and a github account, you can push code onto
a public github branch having also applied the patch mentioned at
https://wiki.postgresql.org/wiki/Continuous_Integration, go to
appveyor.com and tell it to watch your git hub account, and then it'll
build and test every time you push a new tweak.  Takes a few minutes
to get the answer each time you try something, but I have managed to
get things working on Windows that way.


Thanks for the tip.

I'll try that if the blind attempt attached version does not work.

--
Fabien.diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index 25abd0a875..39bba12c23 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -7,7 +7,7 @@ subdir = src/bin/pgbench
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = pgbench.o exprparse.o $(WIN32RES)
+OBJS = pgbench.o exprparse.o symbol_table.o $(WIN32RES)
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
 LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 17c9ec32c5..8c4ad7eff3 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -211,7 +211,7 @@ make_variable(char *varname)
 	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
 
 	expr->etype = ENODE_VARIABLE;
-	expr->u.variable.varname = varname;
+	expr->u.variable.varnum = getOrCreateSymbolId(symbol_table, varname);
 	return expr;
 }
 
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index e9020ad231..1c80e5cac6 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -207,19 +207,19 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 {digit}+		{
 	if (!strtoint64(yytext, true, >ival))
 		expr_yyerror_more(yyscanner, "bigint constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return INTEGER_CONST;
 }
 {digit}+(\.{digit}*)?([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 \.{digit}+([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 {alpha}{alnum}*	{
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ed7652bfbf..dd7f88c136 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -232,24 +232,30 @@ const char *progname;
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /*
- * Variable definitions.
+ * Variable contents.
+ *
+ * Variable names are managed in symbol_table with a number.
  *
  * If a variable only has a string value, "svalue" is that value, and value is
- * "not set".  If the value is known, "value" contains the value (in any
- * variant).
+ * "not set".
+ *
+ * If the value is known, "value" contains the value (in any variant).
  *
  * In this case "svalue" contains the string equivalent of the value, if we've
- * had occasion to compute that, or NULL if we haven't.
+ * had occasion to compute that, or an empty string if we haven't.
  */
+#define SVALUE_SIZE	64
 typedef struct
 {
-	char	   *name;			/* variable's name */
-	char	   *svalue;			/* its value in string form, if known */
-	PgBenchValue value;			/* actual variable's value */
+	int			number;/* variable symbol number, -1 if unset */
+	char			svalue[SVALUE_SIZE];	/* value in string form, if known */
+	PgBenchValue	value;	/* actual value, if known */
 } Variable;
 
+#define undefined_variable_value(v)		\
+	v.svalue[0] == '\0' && v.value.type == PGBT_NO_VALUE
+
 #define MAX_SCRIPTS		128		/* max number of SQL scripts allowed */
-#define SHELL_COMMAND_SIZE	256 /* maximum size allowed for shell command */
 
 /*
  * Simple data structure to keep stats about something.
@@ -395,7 +401,6 @@ typedef struct
 	/* client variables */
 	Variable   *variables;		/* array of variable definitions */
 	int			nvariables;		/* number of variables */
-	bool		vars_sorted;	/* are variables sorted by name? */
 
 	/* various times about current transaction */
 	int64		txn_scheduled;	/* scheduled start time of transaction (usec) */
@@ -408,6 +413,9 @@ typedef struct
 	/* per client collected stats */
 	int64		cnt;			/* client transaction count, for -t */
 	int			ecnt;			/* error count */
+
+	/* next buffer should be at thread level, but it would change functions... */
+	PQExpBufferData	buf;		/* persistant buffer to avoid malloc/free cycles */
 }

Re: pgbench - rework variable management

2019-09-03 Thread Thomas Munro 
On Tue, Sep 3, 2019 at 4:57 PM Fabien COELHO  wrote:
> I noticed, but I do not have any windows host so I cannot test locally.
>
> The issue is how to do a mutex on Windows, which does not have pthread so
> it has to be emulated. I'll try again by sending a blind update to the
> patch and see how it goes.

If you have the patience and a github account, you can push code onto
a public github branch having also applied the patch mentioned at
https://wiki.postgresql.org/wiki/Continuous_Integration, go to
appveyor.com and tell it to watch your git hub account, and then it'll
build and test every time you push a new tweak.  Takes a few minutes
to get the answer each time you try something, but I have managed to
get things working on Windows that way.

-- 
Thomas Munro
https://enterprisedb.com

Re: pgbench - rework variable management

2019-09-02 Thread Fabien COELHO 




Some windows-specific hacks are note tested.


Somehow this macro hackery has upset the Windows socket headers:

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.55019


I noticed, but I do not have any windows host so I cannot test locally.

The issue is how to do a mutex on Windows, which does not have pthread so 
it has to be emulated. I'll try again by sending a blind update to the 
patch and see how it goes.


--
Fabien.

Re: pgbench - rework variable management

2019-09-02 Thread Thomas Munro 
On Wed, Aug 14, 2019 at 3:54 AM Fabien COELHO  wrote:
> Some windows-specific hacks are note tested.

Somehow this macro hackery has upset the Windows socket headers:

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.55019

-- 
Thomas Munro
https://enterprisedb.com

pgbench - rework variable management

2019-08-13 Thread Fabien COELHO 


Hello pgdevs,

The attached patch improves pgbench variable management as discussed in:

https://www.postgresql.org/message-id/flat/alpine.DEB.2.21.1904081752210.5867@lancre

As discussed there as well, the overall effect is small compared to libpq 
& system costs when pgbench is talking to a postgres server. When someone 
says "pgbench is slow", they really mean "libpq &  are slow", 
because pgbench does not do much beyond jumping from one libpq call to the 
next. Anyway, the patch has a measurable positive effect.


###

Rework pgbench variables and associated values for better performance

 - a (hopefully) thread-safe symbol table which maps variable names to integers
   note that all variables are statically known, but \gset stuff.
 - numbers are then used to access per-client arrays

The symbol table stores names as distinct leaves in a tree on bytes.
Each symbol name is the shortest-prefix leaf, possibly including the final
'\0'. Some windows-specific hacks are note tested. File "symbol_table_test.c"
does what it says and can be compiled standalone.

Most malloc/free cycles are taken out of running a benchmark:
 - there is a (large?) maximum number of variables of 32*MAX_SCRIPTS
 - variable names and string  values are statically allocated,
   and limited to, 64 bytes
 - a per-client persistent buffer is used for various purpose,
   to avoid mallocs/frees.

Functions assignVariables & parseQuery basically shared the same variable
substitution logic, but differed in what was substituted. The logic has been
abstracted into a common function.

This patch brings pgbench-specific overheads down on some tests, one 
thread one client, on my laptop, with the attached scripts, in tps:

 - set_x_1.sql: 11.1M -> 14.2M
 - sets.sql: 0.8M -> 2.7M # 20 \set
 - set.sql: 1.5M -> 2.0M # 3 \set & "complex" expressions
 - empty.sql: 63.9K -> 64.1K (…)
 - select_aid.sql: 29.3K -> 29.3K
 - select_aids.sql: 23.4K -> 24.2K
 - gset_aid.sql: 28.3K -> 29.2K

So we are talking significant improvements on pgbench-only scripts, only
a few percents once pgbench must interact with a CPU-bound server, because 
time is spent elsewhere.


--
Fabien.

empty.sql
Description: application/sql


select_aids.sql
Description: application/sql


select_aid.sql
Description: application/sql
diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index 25abd0a875..39bba12c23 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -7,7 +7,7 @@ subdir = src/bin/pgbench
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = pgbench.o exprparse.o $(WIN32RES)
+OBJS = pgbench.o exprparse.o symbol_table.o $(WIN32RES)
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
 LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 17c9ec32c5..8c4ad7eff3 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -211,7 +211,7 @@ make_variable(char *varname)
 	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
 
 	expr->etype = ENODE_VARIABLE;
-	expr->u.variable.varname = varname;
+	expr->u.variable.varnum = getOrCreateSymbolId(symbol_table, varname);
 	return expr;
 }
 
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index e9020ad231..1c80e5cac6 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -207,19 +207,19 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 {digit}+		{
 	if (!strtoint64(yytext, true, >ival))
 		expr_yyerror_more(yyscanner, "bigint constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return INTEGER_CONST;
 }
 {digit}+(\.{digit}*)?([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 \.{digit}+([eE][-+]?{digit}+)?	{
 	if (!strtodouble(yytext, true, >dval))
 		expr_yyerror_more(yyscanner, "double constant overflow",
-		  strdup(yytext));
+		  pg_strdup(yytext));
 	return DOUBLE_CONST;
 }
 {alpha}{alnum}*	{
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 570cf3306a..8025be302d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -232,24 +232,30 @@ const char *progname;
 volatile bool timer_exceeded = false;	/* flag from signal handler */
 
 /*
- * Variable definitions.
+ * Variable contents.
+ *
+ * Variable names are managed in symbol_table with a number.
  *
  * If a variable only has a string value, "svalue" is that value, and value is
- * "not set".  If the value is known, "value" contains the value (in any
- * variant).
+ * "not set".
+ *
+ * If the value is known, "value" contains the value (in any variant).
  *
  * In this case "svalue" contains the string equivalent of the value, if we've
- * had occasion to compute