diff --git a/jOOQ/src/main/java/org/jooq/DiagnosticsContext.java b/jOOQ/src/main/java/org/jooq/DiagnosticsContext.java index 9d1a315a65..26372725ec 100644 --- a/jOOQ/src/main/java/org/jooq/DiagnosticsContext.java +++ b/jOOQ/src/main/java/org/jooq/DiagnosticsContext.java @@ -40,6 +40,7 @@ package org.jooq; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.util.List; +import java.util.Set; /** * A parameter object that is passed to {@link DiagnosticsListener} methods. @@ -123,6 +124,11 @@ public interface DiagnosticsContext { */ int resultSetColumnIndex(); + /** + * The actual statement that is being executed. + */ + String actualStatement(); + /** * The normalised statement that all duplicates correspond to. */ @@ -132,5 +138,11 @@ public interface DiagnosticsContext { * The duplicate statements that all correspond to a single normalised * statement. */ - List duplicateStatements(); + Set duplicateStatements(); + + /** + * The repeated statements that all correspond to a single normalised + * statement. + */ + List repeatedStatements(); } diff --git a/jOOQ/src/main/java/org/jooq/DiagnosticsListener.java b/jOOQ/src/main/java/org/jooq/DiagnosticsListener.java index 9bebff22da..834cb97941 100644 --- a/jOOQ/src/main/java/org/jooq/DiagnosticsListener.java +++ b/jOOQ/src/main/java/org/jooq/DiagnosticsListener.java @@ -37,6 +37,8 @@ */ package org.jooq; +import java.sql.Connection; +import java.sql.PreparedStatement; import java.sql.ResultSet; /** @@ -97,7 +99,7 @@ public interface DiagnosticsListener { * query, but mostly, this is not desirable as calculating execution plans * can turn out to be expensive. *

- * Examples of such similar statements include: + * Examples of such duplicate statements include: *

*

Whitespace differences

*

@@ -120,8 +122,57 @@ public interface DiagnosticsListener { * SELECT * FROM actor a2 WHERE a2.id = ?; * *

- * This event is triggered every time a new duplicate is encountered. + * Examples of identical statements (which are not considered duplicate, but + * {@link #repeatedStatements(DiagnosticsContext)}, if on the same + * {@link Connection}) are: + *

+ * 

+     * SELECT * FROM actor WHERE id = ?;
+     * SELECT * FROM actor WHERE id = ?;
+     *
+ *

+ * This is a system-wide diagnostic that is not specific to individual + * {@link Connection} instances. */ void duplicateStatements(DiagnosticsContext ctx); + /** + * The executed JDBC statement is repeated consecutively on the same JDBC + * {@link Connection}. + *

+ * This problem goes by many names, the most famous one being the N + * + 1 problem, when a single (1) query for a parent entity + * requires many (N) subsequent queries for child entities. This could have + * been prevented by rewriting the parent query to use a JOIN. If such a + * rewrite is not possible (or not easy), the subsequent N queries could at + * least profit (depending on the exact query): + *

    + *
  • From reusing the {@link PreparedStatement}
    • + *
  • From being batched
    • + *
  • From being re-written as a bulk fetch or write query
    • + *
+ *

+ * This problem can be aggravated if combined with the + * {@link #duplicateStatements(DiagnosticsContext)} problem, in case of + * which the repeated statements might not be diagnosed as easily. + *

+ * Repeated statements may or may not be "identical". In the following + * example, there are two repeated and identical statements: + * 

+     * SELECT * FROM actor WHERE id = ?;
+     * SELECT * FROM actor WHERE id = ?;
+     *
+ *

+ * In this example, we have three repeated statements, only some of which + * are also identical: 

+     * SELECT * FROM actor WHERE id = ?;
+     * SELECT * FROM actor WHERE id = ?;
+     * SELECT * FROM actor WHERE id =  ?;
+     *
+ *

+ * This is a {@link Connection}-specific diagnostic that is reset every time + * {@link Connection#close()} is called. + */ + void repeatedStatements(DiagnosticsContext ctx); + } diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsContext.java b/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsContext.java index 8248e75a09..c7097e41ee 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsContext.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsContext.java @@ -39,9 +39,9 @@ package org.jooq.impl; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; import org.jooq.DiagnosticsContext; @@ -55,19 +55,23 @@ final class DefaultDiagnosticsContext implements DiagnosticsContext { int resultSetActualColumns; int resultSetFetchedRows; int resultSetActualRows; + final String actualStatement; final String normalisedStatement; - final List duplicateStatements; + final Set duplicateStatements; + final List repeatedStatements; boolean resultSetUnnecessaryWasNullCall; boolean resultSetMissingWasNullCall; int resultSetColumnIndex; - DefaultDiagnosticsContext(String statement) { - this(statement, Arrays.asList(statement)); + DefaultDiagnosticsContext(String actualStatement) { + this(actualStatement, actualStatement, Collections.singleton(actualStatement), Collections.singletonList(actualStatement)); } - DefaultDiagnosticsContext(String normalisedStatement, List duplicateStatements) { + DefaultDiagnosticsContext(String actualStatement, String normalisedStatement, Set duplicateStatements, List repeatedStatements) { + this.actualStatement = actualStatement; this.normalisedStatement = normalisedStatement; - this.duplicateStatements = duplicateStatements; + this.duplicateStatements = duplicateStatements == null ? Collections.emptySet() : duplicateStatements; + this.repeatedStatements = repeatedStatements == null ? Collections.emptyList() : repeatedStatements; } @Override @@ -120,13 +124,23 @@ final class DefaultDiagnosticsContext implements DiagnosticsContext { return resultSet == null ? 0 : resultSetColumnIndex; } + @Override + public final String actualStatement() { + return actualStatement; + } + @Override public final String normalisedStatement() { return normalisedStatement; } @Override - public final List duplicateStatements() { - return Collections.unmodifiableList(duplicateStatements); + public final Set duplicateStatements() { + return Collections.unmodifiableSet(duplicateStatements); + } + + @Override + public final List repeatedStatements() { + return Collections.unmodifiableList(repeatedStatements); } } diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsListener.java b/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsListener.java index 4938c66764..322740a62c 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsListener.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultDiagnosticsListener.java @@ -65,4 +65,7 @@ public class DefaultDiagnosticsListener implements DiagnosticsListener { @Override public void duplicateStatements(DiagnosticsContext ctx) {} + @Override + public void repeatedStatements(DiagnosticsContext ctx) {} + } diff --git a/jOOQ/src/main/java/org/jooq/impl/DiagnosticsConnection.java b/jOOQ/src/main/java/org/jooq/impl/DiagnosticsConnection.java index e9d7abd88d..2d9166d72b 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DiagnosticsConnection.java +++ b/jOOQ/src/main/java/org/jooq/impl/DiagnosticsConnection.java @@ -64,9 +64,15 @@ import org.jooq.tools.jdbc.DefaultConnection; */ final class DiagnosticsConnection extends DefaultConnection { - static final Map> DUPLICATE_SQL = Collections.synchronizedMap(new LRU()); + // TODO: Make these configurable + static final int LRU_SIZE_GLOBAL = 50000; + static final int LRU_SIZE_LOCAL = 500; + static final int DUP_SIZE = 500; + static final Map> DUPLICATE_SQL = Collections.synchronizedMap(new LRU>(LRU_SIZE_GLOBAL)); + + final Map> repeatedSQL = new LRU>(LRU_SIZE_LOCAL); final Configuration configuration; - final RenderContext duplicates; + final RenderContext normalisingRenderer; final Parser parser; final DiagnosticsListeners listeners; @@ -75,7 +81,7 @@ final class DiagnosticsConnection extends DefaultConnection { super(configuration.connectionProvider().acquire()); this.configuration = configuration; - this.duplicates = configuration.derive( + this.normalisingRenderer = configuration.derive( SettingsTools.clone(configuration.settings()) // Forcing all inline parameters to be indexed helps find opportunities to use bind variables @@ -151,55 +157,79 @@ final class DiagnosticsConnection extends DefaultConnection { @Override public final void close() throws SQLException { + repeatedSQL.clear(); configuration.connectionProvider().release(getDelegate()); } final String parse(String sql) { Queries queries; + String normalised; try { queries = parser.parse(sql); + normalised = normalisingRenderer.render(queries); } catch (ParserException ignore) { - return sql; + normalised = sql; } - String normalised = duplicates.render(queries); - List duplicates = null; - + Set duplicates; synchronized (DUPLICATE_SQL) { - Set v = DUPLICATE_SQL.get(normalised); - - if (v == null) { - v = new HashSet(); - DUPLICATE_SQL.put(normalised, v); - } - - if (v.size() >= DUP_SIZE || (v.add(sql) && v.size() > 1)) - duplicates = new ArrayList(v); + duplicates = duplicates(DUPLICATE_SQL, sql, normalised); } if (duplicates != null) - listeners.duplicateStatements(new DefaultDiagnosticsContext(normalised, duplicates)); + listeners.duplicateStatements(new DefaultDiagnosticsContext(sql, normalised, duplicates, null)); + + List repetitions = repetitions(repeatedSQL, sql, normalised); + + if (repetitions != null) + listeners.repeatedStatements(new DefaultDiagnosticsContext(sql, normalised, null, repetitions)); return sql; } - // TODO: Make this configurable - static final int LRU_SIZE = 50000; - static final int DUP_SIZE = 500; + private Set duplicates(Map> map, String sql, String normalised) { + Set v = map.get(normalised); + + if (v == null) { + v = new HashSet(); + map.put(normalised, v); + } + + if (v.size() >= DUP_SIZE || (v.add(sql) && v.size() > 1)) + return v; + else + return null; + } + + private List repetitions(Map> map, String sql, String normalised) { + List v = map.get(normalised); + + if (v == null) { + v = new ArrayList(); + map.put(normalised, v); + } + + if (v.size() >= DUP_SIZE || (v.add(sql) && v.size() > 1)) + return v; + else + return null; + } // See https://stackoverflow.com/a/1953516/521799 - static class LRU extends LinkedHashMap> { + static class LRU extends LinkedHashMap { private static final long serialVersionUID = 5287799057535876982L; + private final int size; - LRU() { - super(LRU_SIZE + 1, 1.0f, true); + LRU(int size) { + super(size + 1, 1.0f, true); + this.size = size; } @Override - protected boolean removeEldestEntry(Entry> eldest) { - return size() > LRU_SIZE; + protected boolean removeEldestEntry(Entry eldest) { + return size() > size; } } } diff --git a/jOOQ/src/main/java/org/jooq/impl/DiagnosticsListeners.java b/jOOQ/src/main/java/org/jooq/impl/DiagnosticsListeners.java index 6bd5b1e119..68d3a7d4fe 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DiagnosticsListeners.java +++ b/jOOQ/src/main/java/org/jooq/impl/DiagnosticsListeners.java @@ -90,4 +90,10 @@ final class DiagnosticsListeners implements DiagnosticsListener { for (DiagnosticsListener listener : listeners) listener.duplicateStatements(ctx); } + + @Override + public final void repeatedStatements(DiagnosticsContext ctx) { + for (DiagnosticsListener listener : listeners) + listener.repeatedStatements(ctx); + } }