Message Title

Having the Session implement CTC is not "clean", I definitely agree with that. The point of that suggestion was to handle your second point.. to minimize instantiations/allocations of some separate "no txn" CTC delegate. Personally I think that the performance gain here is going to be more important than the "messiness".

Right. The "Session is a CacheTransactionContext" option is maybe a bit hard to visualize, so perhaps some code would help:

/**
 * ...
 * 
 * @apiNote Note that Session implements CacheTransactionContext for cases where either
 * L2C is not enabled or when L2C is enabled but we are not joined to a txn context.  In this
 * case, the Session itself will act as the CacheTransactionContext for calls to the cache.  This
 * is done to minimize instantiations of a separate CacheTransactionContext delegate for these 
 * cases which would get expensive resource-wise.
 */
class SessionImpl ... implements ..., CacheTransactionContext {
    ...
    private CacheTransactionContext explicitCurrentCacheTransactionContext;
    private long nonTransactedStartTimestamp;

    public SessionImpl(...) {
        ...
        resetNonTransactedStartTimestamp();
    }

    private void resetNonTransactedStartTimestamp() {
         this.nonTransactedStartTimestamp = sessionFactory.getCacheEngine().getRegionFactory().nextTimestamp();
    }

    @Override
    public CacheTransactionContext getCurrentCacheTransactionContext() {
        return explicitCurrentCacheTransactionContext == null
                ? this
                : explicitCurrentCacheTransactionContext;
    }

    /**
     * ...
     * @implNote this method is called whenever we join a txn context.  This will happen when either
     * a JDBC-based transaction is started via the Hibernate Transaction API or we join a JTA-based
     * transaction.
     */
    @Override
    public void startTransactionBoundary() {
        this.cacheTransactionContext = factory.getCache().getRegionFactory().startingTransaction( this );
    }

    @Override
    public void beforeTransactionCompletion() {
        getCurrentCacheTransactionContext().transactionCompleting();
        ...
    }

    @Override
    public void afterTransactionCompletion(boolean successful, boolean delayed) {
        ...
        getCurrentCacheTransactionContext().transactionCompleted(); // anything to pass?
        this.explicitCurrentCacheTransactionContext = null;
        resetNonTransactedStartTimestamp();
    }

    // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    // CacheTransactionContext implementation
    
    @Override
    public long getCurrentTransactionStartTimestamp() {
        return nonTransactedStartTimestamp;
    }

    @Override 
    public void beforeCompletion() {
        // nothing to do here
    }

    @Override 
    public void afterCompletion() {
        // nothing to do here
    }
}

Again, I agree this is "messy" - it is a matter of performance versus generating a new CacheTransactionContext instance for every call to "resetNonTransactedStartTimestamp".

The other option I mentioned is to have that single CacheTransactionContext instance be in effect for the entire Session lifecycle, something like:

class SessionImpl ... {
    private CacheTransactionContext cacheTransactionContext;

    public SessionImpl(...) {
        this.cacheTransactionContext = factory.getCache().getRegionFactory().startingTransaction( this );
    }

    @Override
    public CacheTransactionContext getCurrentCacheTransactionContext() {
        return cacheTransactionContext;
    }

    @Override
    public void startTransactionBoundary() {
        // potentially, if deemed useful
        getCurrentCacheTransactionContext().transactionJoined();
    }

    @Override
    public void beforeTransactionCompletion() {
        ...
        getCurrentCacheTransactionContext().transactionCompleting();
        ...
    }

    @Override
    public void afterTransactionCompletion(boolean successful, boolean delayed) {
        ...
        getCurrentCacheTransactionContext().transactionCompleted(); // anything to pass?
        ...
    }

}

here you can see that the CacheTransactionContext is given enough contextual callbacks to be aware of transaction boundaries.

As far as "real" caching calls, this is accurate. However, we'd also have to perform this when a user makes calls of the "caching engine" which happens:

In these cases we could either generate the Name each time, or we could decide to maintain a map of "raw" region name to its Name reference. The latter is probably a premature optimization though at this point.