Re: ResourceOwner refactoring - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: ResourceOwner refactoring
Date
Msg-id 20210118143443.GA7276@alvherre.pgsql
Whole thread Raw
In response to Re: ResourceOwner refactoring  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: ResourceOwner refactoring
Re: ResourceOwner refactoring
List pgsql-hackers
On 2021-Jan-18, Heikki Linnakangas wrote:

> +static ResourceOwnerFuncs jit_funcs =
> +{
> +    /* relcache references */
> +    .name = "LLVM JIT context",
> +    .phase = RESOURCE_RELEASE_BEFORE_LOCKS,
> +    .ReleaseResource = ResOwnerReleaseJitContext,
> +    .PrintLeakWarning = ResOwnerPrintJitContextLeakWarning
> +};

I think you mean jit_resowner_funcs here; "jit_funcs" is a bit
excessively vague.  Also, why did you choose not to define
ResourceOwnerRememberJIT?  You do that in other modules and it seems
better.

> +static ResourceOwnerFuncs catcache_funcs =
> +{
> +    /* catcache references */
> +    .name = "catcache reference",
> +    .phase = RESOURCE_RELEASE_AFTER_LOCKS,
> +    .ReleaseResource = ResOwnerReleaseCatCache,
> +    .PrintLeakWarning = ResOwnerPrintCatCacheLeakWarning
> +};
> +
> +static ResourceOwnerFuncs catlistref_funcs =
> +{
> +    /* catcache-list pins */
> +    .name = "catcache list reference",
> +    .phase = RESOURCE_RELEASE_AFTER_LOCKS,
> +    .ReleaseResource = ResOwnerReleaseCatCacheList,
> +    .PrintLeakWarning = ResOwnerPrintCatCacheListLeakWarning
> +};

Similar naming issue here, I say.


> diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c
> index 551ec392b60..642e72d8c0f 100644
> --- a/src/common/cryptohash_openssl.c
> +++ b/src/common/cryptohash_openssl.c
> @@ -60,6 +59,21 @@ struct pg_cryptohash_ctx
>  #endif
>  };
>  
> +/* ResourceOwner callbacks to hold JitContexts  */
> +#ifndef FRONTEND
> +static void ResOwnerReleaseCryptoHash(Datum res);
> +static void ResOwnerPrintCryptoHashLeakWarning(Datum res);

The comment is wrong -- "Crypohashes", not "JitContexts".


So according to your performance benchmark, we're willing to accept a
30% performance loss on an allegedly common operation -- numkeep=0
numsnaps=10 becomes 49.8ns from 37.6ns.  That seems a bit shocking.
Maybe you can claim that these operations aren't exactly hot spots, and
so the fact that we remain in the same power-of-ten is sufficient.  Is
that the argument?

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"The Postgresql hackers have what I call a "NASA space shot" mentality.
 Quite refreshing in a world of "weekend drag racer" developers."
(Scott Marlowe)



pgsql-hackers by date:

Previous
From: Michael Banck
Date:
Subject: Re: recovering from "found xmin ... from before relfrozenxid ..."
Next
From: Robert Haas
Date:
Subject: Re: recovering from "found xmin ... from before relfrozenxid ..."