Horton

Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@postgresql.org, Nikita Glukhov <n.gluhov@postgrespro.ru>, Andrew Dunstan <andrew@dunslane.net>, David Rowley <dgrowleyml@gmail.com>
Date: 2023-02-23T19:35:09Z
Lists: pgsql-hackers 
Hi,

On 2023-02-23 13:39:14 -0500, Corey Huinker wrote:
> My not-ready-for-16 work on CAST( ... ON DEFAULT ... ) involved making
> FuncExpr/IoCoerceExpr/ArrayCoerceExpr have a safe_mode flag, and that
> necessitates adding a reserror boolean to ExprEvalStep for subsequent steps
> to test if the error happened.

I think if that requires adding a new variable to each ExprEvalStep, it's
DOA. The overhead would be too high. But I don't see why it would need to be
added all ExprEvalSteps instead of individual steps, or perhaps ExprEvalState?


> Will that change be throwing some architectures over the 64 byte count?

It would.

I find the 'pahole' tool very useful for looking at struct layout.


struct ExprEvalStep {
        intptr_t                   opcode;               /*     0     8 */
        Datum *                    resvalue;             /*     8     8 */
        _Bool *                    resnull;              /*    16     8 */
        union {
                struct {
                        int        last_var;             /*    24     4 */
                        _Bool      fixed;                /*    28     1 */

                        /* XXX 3 bytes hole, try to pack */

                        TupleDesc  known_desc;           /*    32     8 */
                        const TupleTableSlotOps  * kind; /*    40     8 */
                } fetch;                                 /*    24    24 */
...
                struct {
                        Datum *    values;               /*    24     8 */
                        _Bool *    nulls;                /*    32     8 */
                        int        nelems;               /*    40     4 */
                        MinMaxOp   op;                   /*    44     4 */
                        FmgrInfo * finfo;                /*    48     8 */
                        FunctionCallInfo fcinfo_data;    /*    56     8 */
                } minmax;                                /*    24    40 */
...

        } d;                                             /*    24    40 */

        /* size: 64, cachelines: 1, members: 4 */
};


We don't have memory to spare in the "general" portion of ExprEvalStep
(currently 24 bytes), as several of the type-specific portions are already 40
bytes large.

Greetings,

Andres Freund

Commits

  1. JSON_TABLE: Add support for NESTED paths and columns

  2. Add basic JSON_TABLE() functionality

  3. Add SQL/JSON query functions

  4. Add soft error handling to some expression nodes

  5. Adjust populate_record_field() to handle errors softly

  6. Refactor code used by jsonpath executor to fetch variables

  7. Add more SQL/JSON constructor functions

  8. SQL/JSON: support the IS JSON predicate

  9. SQL/JSON: add standard JSON constructor functions

  10. Add static assertion ensuring sizeof(ExprEvalStep) <= 64 bytes

  11. Remove size increase in ExprEvalStep caused by hashed saops

  12. pgstat: reduce timer overhead by leaving timer running.

  13. expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size.

  14. SQL/JSON query functions

  15. Speedup ScalarArrayOpExpr evaluation