Re: svn commit: r1737349 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBConnectionHandler.java

[Chetan Mehrotra <ch...@gmail.com>]

Hi Julian,

On Fri, Apr 1, 2016 at 5:19 PM,  <re...@apache.org> wrote:
> +    @Nonnull
> +    private Connection getConnection() throws IllegalStateException, SQLException {
> +        long ts = System.currentTimeMillis();
> +        Connection c = getDataSource().getConnection();
> +        if (LOG.isDebugEnabled()) {
> +            long elapsed = System.currentTimeMillis() - ts;
> +            if (elapsed >= 100) {
> +                LOG.debug("Obtaining a new connection from " + this.ds + " took " + elapsed + "ms");
> +            }
> +        }
> +        return c;
> +    }

You can also use PerfLogger here which is also used in other places in
DocumentNodeStore

---
final PerfLogger PERFLOG = new PerfLogger(
            LoggerFactory.getLogger(DocumentNodeStore.class.getName()
+ ".perf"));

final long start = PERFLOG.start();
Connection c = getDataSource().getConnection();
PERFLOG.end(start, 100, "Obtaining a new connection from {} ", ds);
---

This would also avoid the call to System.currentTimeMillis() if debug
log is not enabled

Chetan Mehrotra

Re: svn commit: r1737349 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBConnectionHandler.java

[Chetan Mehrotra <ch...@gmail.com>]

On Fri, Apr 1, 2016 at 6:40 PM, Julian Reschke <ju...@gmx.de> wrote:
> Did you benchmark System.currentTimeMillis() as opposed to checking the log
> level?

Well time taken by single isDebugEnabled would always be less than
System.currentTimeMillis()  + isDebugEnabled! In this case it anyway
does not matter much as remote call would have much more overhead.

Suggestion here was more to have a consistent way of doing such things
but not a hard requirement per se ...

Chetan Mehrotra

Re: svn commit: r1737349 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBConnectionHandler.java

[Julian Reschke <ju...@gmx.de>]

On 2016-04-01 14:48, Chetan Mehrotra wrote:
> ...
> This would also avoid the call to System.currentTimeMillis() if debug
> log is not enabled
> ...

Did you benchmark System.currentTimeMillis() as opposed to checking the 
log level?

/me took the simple approach here...