From 9e19c8871de29809cbba8f4a37984ea603b32dcc Mon Sep 17 00:00:00 2001 From: srosse <stephane.rosse@frentix.com> Date: Sat, 27 Oct 2018 20:48:53 +0200 Subject: [PATCH] OO-3584: optimize filterMembership query (factor 100 increase in speed) --- .../java/org/olat/core/util/CodeHelper.java | 13 +++++++---- .../manager/BusinessGroupServiceImpl.java | 6 +---- .../modules/coach/manager/CoachingDAO.java | 2 ++ .../RepositoryEntryMyCourseQueries.java | 8 +++++-- .../manager/RepositoryEntryRelationDAO.java | 18 +++++++-------- .../model/RepositoryEntryToGroupRelation.java | 10 ++++---- .../coach/manager/CoachingDAOTest.java | 6 ++--- .../RepositoryEntryRelationDAOTest.java | 23 +++++++++++++++++++ .../accesscontrol/ACFrontendManagerTest.java | 2 +- .../accesscontrol/ACOrderManagerTest.java | 4 ++-- 10 files changed, 60 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/olat/core/util/CodeHelper.java b/src/main/java/org/olat/core/util/CodeHelper.java index ca9f652f9b5..0553412c201 100644 --- a/src/main/java/org/olat/core/util/CodeHelper.java +++ b/src/main/java/org/olat/core/util/CodeHelper.java @@ -106,13 +106,18 @@ public class CodeHelper { public static long nanoToMilliTime(long start) { long end = System.nanoTime(); - long takes = (end - start) / 1000000; - return takes; + return (end - start) / 1000000l; } - public static void printNanoTime(long start, String action) { + public static void printMilliSecondTime(long nanoStart, String action) { long end = System.nanoTime(); - long takes = (end - start) / 1000000; + long takes = (end - nanoStart) / 1000000l; System.out.println(action + " takes (ms): " + takes); } + + public static void printMicroSecondTime(long nanoStart, String action) { + long end = System.nanoTime(); + long takes = (end - nanoStart) / 1000l; + System.out.println(action + " takes (\u00B5s): " + takes); + } } diff --git a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java index 75e9c55364c..10d8e3b2a91 100644 --- a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java +++ b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java @@ -55,7 +55,6 @@ import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; import org.olat.core.logging.activity.ActionType; import org.olat.core.logging.activity.ThreadLocalUserActivityLogger; -import org.olat.core.util.CodeHelper; import org.olat.core.util.async.ProgressDelegate; import org.olat.core.util.coordinate.CoordinatorManager; import org.olat.core.util.mail.ContactList; @@ -1783,10 +1782,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { @Override public List<Identity> getIdentitiesWithRole(String role) { - long start = System.nanoTime(); - List<Identity> ids = businessGroupRelationDAO.getIdentitiesWithRole(role); - CodeHelper.printNanoTime(start, "Group with role"); - return ids; + return businessGroupRelationDAO.getIdentitiesWithRole(role); } @Override diff --git a/src/main/java/org/olat/modules/coach/manager/CoachingDAO.java b/src/main/java/org/olat/modules/coach/manager/CoachingDAO.java index b93270cff2e..148c6161863 100644 --- a/src/main/java/org/olat/modules/coach/manager/CoachingDAO.java +++ b/src/main/java/org/olat/modules/coach/manager/CoachingDAO.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import javax.persistence.FlushModeType; import javax.persistence.Query; import org.olat.basesecurity.GroupRoles; @@ -89,6 +90,7 @@ public class CoachingDAO { List<Long> firstKey = dbInstance.getCurrentEntityManager() .createQuery(sb.toString(), Long.class) + .setFlushMode(FlushModeType.COMMIT) .setParameter("identityKey", coach.getKey()) .setFirstResult(0) .setMaxResults(1) diff --git a/src/main/java/org/olat/repository/manager/RepositoryEntryMyCourseQueries.java b/src/main/java/org/olat/repository/manager/RepositoryEntryMyCourseQueries.java index cc8201bcc4c..22d925d8e1f 100644 --- a/src/main/java/org/olat/repository/manager/RepositoryEntryMyCourseQueries.java +++ b/src/main/java/org/olat/repository/manager/RepositoryEntryMyCourseQueries.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.persistence.FlushModeType; import javax.persistence.TypedQuery; import org.olat.basesecurity.GroupRoles; @@ -87,7 +88,9 @@ public class RepositoryEntryMyCourseQueries { } TypedQuery<Number> query = creatMyViewQuery(params, Number.class); - Number count = query.getSingleResult(); + Number count = query + .setFlushMode(FlushModeType.COMMIT) + .getSingleResult(); return count == null ? 0 : count.intValue(); } @@ -98,7 +101,8 @@ public class RepositoryEntryMyCourseQueries { } TypedQuery<Object[]> query = creatMyViewQuery(params, Object[].class); - query.setFirstResult(firstResult); + query.setFlushMode(FlushModeType.COMMIT) + .setFirstResult(firstResult); if(maxResults > 0) { query.setMaxResults(maxResults); } diff --git a/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java b/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java index a82e3cf6f61..3940435d4bc 100644 --- a/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java +++ b/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java @@ -32,6 +32,7 @@ import java.util.Set; import java.util.stream.Collectors; import javax.persistence.EntityManager; +import javax.persistence.FlushModeType; import javax.persistence.TypedQuery; import org.olat.basesecurity.Group; @@ -76,13 +77,12 @@ public class RepositoryEntryRelationDAO { */ public List<String> getRoles(IdentityRef identity, RepositoryEntryRef re) { StringBuilder sb = new StringBuilder(512); - sb.append("select membership.role from repositoryentry as v") - .append(" inner join v.groups as relGroup") + sb.append("select membership.role from repoentrytogroup as relGroup") .append(" inner join relGroup.group as baseGroup") .append(" inner join baseGroup.members as membership") .append(" left join businessgroup as businessGroup on (businessGroup.baseGroup.key=baseGroup.key)") .append(" left join curriculumelement as curEl on (curEl.group.key=baseGroup.key)") - .append(" where v.key=:repoKey and membership.identity.key=:identityKey and (relGroup.defaultGroup=true or businessGroup.key is not null or curEl is not null)"); + .append(" where relGroup.entry.key=:repoKey and membership.identity.key=:identityKey and (relGroup.defaultGroup=true or businessGroup.key is not null or curEl is not null)"); return dbInstance.getCurrentEntityManager() .createQuery(sb.toString(), String.class) @@ -101,12 +101,11 @@ public class RepositoryEntryRelationDAO { public List<Object[]> getRoleAndDefaults(IdentityRef identity, RepositoryEntryRef re) { StringBuilder sb = new StringBuilder(512); sb.append("select membership.role, relGroup.defaultGroup, curEl.key") - .append(" from repositoryentry as v") - .append(" inner join v.groups as relGroup") + .append(" from repoentrytogroup as relGroup") .append(" inner join relGroup.group as baseGroup") .append(" inner join baseGroup.members as membership") .append(" left join curriculumelement curEl on (curEl.group.key=baseGroup.key)") - .append(" where v.key=:repoKey and membership.identity.key=:identityKey"); + .append(" where relGroup.entry.key=:repoKey and membership.identity.key=:identityKey"); return dbInstance.getCurrentEntityManager() .createQuery(sb.toString(), Object[].class) .setParameter("identityKey", identity.getKey()) @@ -276,13 +275,14 @@ public class RepositoryEntryRelationDAO { */ public void filterMembership(IdentityRef identity, Collection<Long> entries) { if(entries == null || entries.isEmpty()) return; - + List<Object[]> membershipList = dbInstance.getCurrentEntityManager() - .createNamedQuery("filterRepositoryEntryMembership", Object[].class) + .createNamedQuery("filterRepositoryEntryRelationMembership", Object[].class) .setParameter("identityKey", identity.getKey()) .setParameter("repositoryEntryKey", entries) + .setFlushMode(FlushModeType.COMMIT) .getResultList(); - + Set<Object> memberships = new HashSet<>(); for(Object[] membership: membershipList) { memberships.add(membership[0]); diff --git a/src/main/java/org/olat/repository/model/RepositoryEntryToGroupRelation.java b/src/main/java/org/olat/repository/model/RepositoryEntryToGroupRelation.java index 9156428e72a..825e281ea7c 100644 --- a/src/main/java/org/olat/repository/model/RepositoryEntryToGroupRelation.java +++ b/src/main/java/org/olat/repository/model/RepositoryEntryToGroupRelation.java @@ -28,7 +28,6 @@ import javax.persistence.GeneratedValue; import javax.persistence.Id; import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; -import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; import javax.persistence.Table; import javax.persistence.Temporal; @@ -49,11 +48,10 @@ import org.olat.repository.RepositoryEntry; */ @Entity(name="repoentrytogroup") @Table(name="o_re_to_group") -@NamedQueries({ - @NamedQuery(name="relationByRepositoryEntryAndGroup",query="select rel from repoentrytogroup as rel where rel.entry.key=:repoKey and rel.group.key=:groupKey"), - @NamedQuery(name="relationByRepositoryEntry", query="select rel from repoentrytogroup as rel where rel.entry.key=:repoKey"), - @NamedQuery(name="relationByGroup", query="select rel from repoentrytogroup as rel where rel.group.key=:groupKey") -}) +@NamedQuery(name="relationByRepositoryEntryAndGroup",query="select rel from repoentrytogroup as rel where rel.entry.key=:repoKey and rel.group.key=:groupKey") +@NamedQuery(name="relationByRepositoryEntry", query="select rel from repoentrytogroup as rel where rel.entry.key=:repoKey") +@NamedQuery(name="relationByGroup", query="select rel from repoentrytogroup as rel where rel.group.key=:groupKey") +@NamedQuery(name="filterRepositoryEntryRelationMembership", query="select relGroup.entry.key, membership.identity.key from repoentrytogroup as relGroup inner join bgroupmember as membership on (relGroup.group.key=membership.group.key) where membership.identity.key=:identityKey and membership.role in ('owner','coach','participant') and relGroup.entry.key in (:repositoryEntryKey)") public class RepositoryEntryToGroupRelation implements Persistable { private static final long serialVersionUID = 2215547264646107606L; diff --git a/src/test/java/org/olat/modules/coach/manager/CoachingDAOTest.java b/src/test/java/org/olat/modules/coach/manager/CoachingDAOTest.java index 79cd11284e1..e0c1c2e1608 100644 --- a/src/test/java/org/olat/modules/coach/manager/CoachingDAOTest.java +++ b/src/test/java/org/olat/modules/coach/manager/CoachingDAOTest.java @@ -1028,15 +1028,15 @@ public class CoachingDAOTest extends OlatTestCase { if(coach != null) { long start = System.nanoTime(); coachingDAO.getCoursesStatisticsNative(coach); - CodeHelper.printNanoTime(start, "Courses"); + CodeHelper.printMilliSecondTime(start, "Courses"); start = System.nanoTime(); coachingDAO.getGroupsStatisticsNative(coach); - CodeHelper.printNanoTime(start, "Groups"); + CodeHelper.printMilliSecondTime(start, "Groups"); start = System.nanoTime(); coachingDAO.getStudentsStatisticsNative(coach, userPropertyHandlers); - CodeHelper.printNanoTime(start, "Students"); + CodeHelper.printMilliSecondTime(start, "Students"); } } } \ No newline at end of file diff --git a/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java b/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java index 2e186e1b79b..1a80c498d79 100644 --- a/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java +++ b/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java @@ -25,6 +25,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; import org.olat.basesecurity.Group; @@ -134,6 +135,28 @@ public class RepositoryEntryRelationDAOTest extends OlatTestCase { Assert.assertEquals(GroupRoles.owner.name(), ownerRoles.get(0)); } + @Test + public void getRoleAndDefaults() { + Identity id = JunitTestHelper.createAndPersistIdentityAsRndUser("get-roles-1-"); + Organisation defOrganisation = organisationService.getDefaultOrganisation(); + RepositoryEntry re = repositoryService.create(null, "Rei Ayanami", "rel", "rel", null, null, + RepositoryEntryStatusEnum.trash, defOrganisation); + dbInstance.commit(); + repositoryEntryRelationDao.addRole(id, re, GroupRoles.participant.name()); + dbInstance.commit(); + + List<Object[]> participantRoles = repositoryEntryRelationDao.getRoleAndDefaults(id, re); + Assert.assertNotNull(participantRoles); + Assert.assertEquals(2, participantRoles.size()); + + Object[] firstRole = participantRoles.get(0); + Object[] secondRole = participantRoles.get(1); + Assert.assertThat((String)firstRole[0], Matchers + .either(Matchers.is(GroupRoles.participant.name())).or(Matchers.is(OrganisationRoles.user.name()))); + Assert.assertThat((String)secondRole[0], Matchers + .either(Matchers.is(GroupRoles.participant.name())).or(Matchers.is(OrganisationRoles.user.name()))); + } + @Test public void removeRole() { Identity id = JunitTestHelper.createAndPersistIdentityAsRndUser("add-role-4-"); diff --git a/src/test/java/org/olat/resource/accesscontrol/ACFrontendManagerTest.java b/src/test/java/org/olat/resource/accesscontrol/ACFrontendManagerTest.java index a0027941e8d..c7ba38d2602 100644 --- a/src/test/java/org/olat/resource/accesscontrol/ACFrontendManagerTest.java +++ b/src/test/java/org/olat/resource/accesscontrol/ACFrontendManagerTest.java @@ -367,7 +367,7 @@ public class ACFrontendManagerTest extends OlatTestCase { Assert.assertNotNull(acResult); Assert.assertTrue(acResult.isAccessible()); dbInstance.commit(); - CodeHelper.printNanoTime(start, "One click"); + CodeHelper.printMilliSecondTime(start, "One click"); } @Test diff --git a/src/test/java/org/olat/resource/accesscontrol/ACOrderManagerTest.java b/src/test/java/org/olat/resource/accesscontrol/ACOrderManagerTest.java index 87f09f8bcc6..d36430c2af0 100644 --- a/src/test/java/org/olat/resource/accesscontrol/ACOrderManagerTest.java +++ b/src/test/java/org/olat/resource/accesscontrol/ACOrderManagerTest.java @@ -197,7 +197,7 @@ public class ACOrderManagerTest extends OlatTestCase { long start = System.nanoTime(); List<RawOrderItem> items = acOrderManager.findNativeOrderItems(randomOres, null, null, null, null, null, 0, -1, null); - CodeHelper.printNanoTime(start, "Order itemized"); + CodeHelper.printMilliSecondTime(start, "Order itemized"); Assert.assertNotNull(items); //check the order by @@ -232,7 +232,7 @@ public class ACOrderManagerTest extends OlatTestCase { long start = System.nanoTime(); List<RawOrderItem> items = acOrderManager.findNativeOrderItems(randomOres, null, null, null, null, null, 0, -1, null); - CodeHelper.printNanoTime(start, "Order itemized"); + CodeHelper.printMilliSecondTime(start, "Order itemized"); Assert.assertNotNull(items); } -- GitLab