From 71250ba8609cbf72fb0c07ff144f0ed6fbfaf750 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 31 May 2013 17:09:31 +0200 Subject: [PATCH] SONAR-4360 optimize memory when loading open issues --- .../core/issue/InitialOpenIssuesSensor.java | 15 +++-- .../core/issue/InitialOpenIssuesStack.java | 50 +++++++--------- .../core/issue/IssueTrackingDecorator.java | 5 +- .../issue/InitialOpenIssuesSensorTest.java | 11 +--- .../issue/InitialOpenIssuesStackTest.java | 58 ++++++++++++------- .../issue/IssueTrackingDecoratorTest.java | 11 ++-- .../org/sonar/core/issue/db/IssueDao.java | 7 ++- .../org/sonar/core/issue/db/IssueDto.java | 11 ++-- .../org/sonar/core/issue/db/IssueDaoTest.java | 14 +++-- 9 files changed, 99 insertions(+), 83 deletions(-) diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java index a0b9f41e94f..9796c39293b 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java @@ -19,15 +19,14 @@ */ package org.sonar.plugins.core.issue; +import org.apache.ibatis.session.ResultContext; +import org.apache.ibatis.session.ResultHandler; import org.sonar.api.batch.Sensor; import org.sonar.api.batch.SensorContext; import org.sonar.api.resources.Project; import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; -import java.util.Date; -import java.util.List; - /** * Load all the issues referenced during the previous scan. */ @@ -48,9 +47,13 @@ public class InitialOpenIssuesSensor implements Sensor { @Override public void analyse(Project project, SensorContext context) { - Date loadingDate = new Date(); - List dtos = issueDao.selectNonClosedIssuesByModule(project.getId()); - initialOpenIssuesStack.setIssues(dtos, loadingDate); + issueDao.selectNonClosedIssuesByModule(project.getId(), new ResultHandler() { + @Override + public void handleResult(ResultContext rc) { + IssueDto dto = (IssueDto) rc.getResultObject(); + initialOpenIssuesStack.addIssue(dto); + } + }); } @Override diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java index 0c4f40af658..d3a4c969c27 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java @@ -20,50 +20,44 @@ package org.sonar.plugins.core.issue; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ListMultimap; +import com.google.common.collect.Lists; import org.sonar.api.BatchExtension; +import org.sonar.api.batch.InstantiationStrategy; +import org.sonar.batch.index.Cache; +import org.sonar.batch.index.Caches; import org.sonar.core.issue.db.IssueDto; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; import java.util.List; +@InstantiationStrategy(InstantiationStrategy.PER_BATCH) public class InitialOpenIssuesStack implements BatchExtension { - private final ListMultimap issuesByResourceId; + private final Cache cache; - private Date loadedDate; - - public InitialOpenIssuesStack() { - issuesByResourceId = ArrayListMultimap.create(); + public InitialOpenIssuesStack(Caches caches) { + cache = caches.createCache("last-open-issues"); } - public void setIssues(List issues, Date loadedDate) { - this.loadedDate = loadedDate; - for (IssueDto issueDto : issues) { - issuesByResourceId.put(issueDto.getComponentId(), issueDto); - } + public InitialOpenIssuesStack addIssue(IssueDto issueDto) { + cache.put(issueDto.getComponentKey(), issueDto.getKee(), issueDto); + return this; } - public List selectAndRemove(final Integer resourceId) { - List foundIssuesDto = issuesByResourceId.get(resourceId); - if (!foundIssuesDto.isEmpty()) { - List issuesDto = ImmutableList.copyOf(foundIssuesDto); - issuesByResourceId.removeAll(resourceId); - return issuesDto; - } else { - return Collections.emptyList(); + public List selectAndRemove(String componentKey) { + Iterable issues = cache.values(componentKey); + List result = Lists.newArrayList(); + for (IssueDto issue : issues) { + result.add(issue); } + cache.clear(componentKey); + return result; } - public Collection getAllIssues() { - return issuesByResourceId.values(); + public Iterable selectAll() { + return cache.allValues(); } - public Date getLoadedDate() { - return loadedDate; + public void clear() { + cache.clearAll(); } } diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 68a90e91629..8e4e3a36de7 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -98,7 +98,7 @@ public class IssueTrackingDecorator implements Decorator { // issues = all the issues created by rule engines during this module scan and not excluded by filters // all the issues that are not closed in db before starting this module scan, including manual issues - Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getId()); + Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getEffectiveKey()); IssueTrackingResult trackingResult = tracking.track(resource, dbOpenIssues, issues); @@ -166,11 +166,12 @@ public class IssueTrackingDecorator implements Decorator { } private void addIssuesOnDeletedComponents(Collection issues) { - for (IssueDto deadDto : initialOpenIssues.getAllIssues()) { + for (IssueDto deadDto : initialOpenIssues.selectAll()) { DefaultIssue dead = deadDto.toDefaultIssue(); updateUnmatchedIssue(dead, true); issues.add(dead); } + initialOpenIssues.clear(); } private void updateUnmatchedIssue(DefaultIssue issue, boolean forceEndOfLife) { diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java index 8a91e329f3c..814da65f220 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java @@ -17,38 +17,33 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - package org.sonar.plugins.core.issue; +import org.apache.ibatis.session.ResultHandler; import org.junit.Test; import org.sonar.api.resources.Project; import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; -import java.util.Date; - import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyListOf; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; - public class InitialOpenIssuesSensorTest { InitialOpenIssuesStack stack = mock(InitialOpenIssuesStack.class); IssueDao issueDao = mock(IssueDao.class); InitialOpenIssuesSensor sensor = new InitialOpenIssuesSensor(stack, issueDao); - @Test public void should_select_module_open_issues() { Project project = new Project("key"); project.setId(1); sensor.analyse(project, null); - verify(issueDao).selectNonClosedIssuesByModule(1); - verify(stack).setIssues(anyListOf(IssueDto.class), any(Date.class)); + verify(issueDao).selectNonClosedIssuesByModule(eq(1), any(ResultHandler.class)); } @Test diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java index df081dd3582..51c8597b778 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java @@ -20,11 +20,12 @@ package org.sonar.plugins.core.issue; +import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.sonar.batch.index.Caches; import org.sonar.core.issue.db.IssueDto; -import java.util.Date; import java.util.List; import static com.google.common.collect.Lists.newArrayList; @@ -32,48 +33,63 @@ import static org.fest.assertions.Assertions.assertThat; public class InitialOpenIssuesStackTest { - private InitialOpenIssuesStack initialOpenIssuesStack; + InitialOpenIssuesStack stack; + Caches caches = new Caches(); @Before - public void before() { - initialOpenIssuesStack = new InitialOpenIssuesStack(); + public void setUp() { + caches.start(); + stack = new InitialOpenIssuesStack(caches); + } + + @After + public void tearDown() { + caches.stop(); } @Test public void should_get_and_remove() { - Date loadedDate = new Date(); - IssueDto issueDto = new IssueDto().setComponentId(10).setId(1L); - initialOpenIssuesStack.setIssues(newArrayList(issueDto), loadedDate); + IssueDto issueDto = new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1"); + stack.addIssue(issueDto); - List issueDtos = initialOpenIssuesStack.selectAndRemove(10); + List issueDtos = stack.selectAndRemove("org.struts.Action"); assertThat(issueDtos).hasSize(1); - assertThat(issueDtos.get(0).getId()).isEqualTo(1L); + assertThat(issueDtos.get(0).getKee()).isEqualTo("ISSUE-1"); - assertThat(initialOpenIssuesStack.getAllIssues()).isEmpty(); - assertThat(initialOpenIssuesStack.getLoadedDate()).isEqualTo(loadedDate); + assertThat(stack.selectAll()).isEmpty(); } @Test public void should_get_and_remove_with_many_issues_on_same_resource() { - initialOpenIssuesStack.setIssues(newArrayList( - new IssueDto().setComponentId(10).setId(1L), - new IssueDto().setComponentId(10).setId(2L) - ), new Date()); + stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); + stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-2")); - List issueDtos = initialOpenIssuesStack.selectAndRemove(10); + List issueDtos = stack.selectAndRemove("org.struts.Action"); assertThat(issueDtos).hasSize(2); - assertThat(initialOpenIssuesStack.getAllIssues()).isEmpty(); + assertThat(stack.selectAll()).isEmpty(); } @Test public void should_do_nothing_if_resource_not_found() { - IssueDto issueDto = new IssueDto().setComponentId(10).setId(1L); - initialOpenIssuesStack.setIssues(newArrayList(issueDto), new Date()); + stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); - List issueDtos = initialOpenIssuesStack.selectAndRemove(999); + List issueDtos = stack.selectAndRemove("Other"); assertThat(issueDtos).hasSize(0); - assertThat(initialOpenIssuesStack.getAllIssues()).hasSize(1); + assertThat(stack.selectAll()).hasSize(1); + } + + @Test + public void should_clear() { + stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); + + assertThat(stack.selectAll()).hasSize(1); + + // issues are not removed + assertThat(stack.selectAll()).hasSize(1); + + stack.clear(); + assertThat(stack.selectAll()).hasSize(0); } } diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index c93f7365d70..627f3c220e0 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -41,7 +41,10 @@ import org.sonar.core.issue.workflow.IssueWorkflow; import org.sonar.core.persistence.AbstractDaoTestCase; import org.sonar.java.api.JavaClass; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.*; @@ -56,13 +59,11 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueWorkflow workflow = mock(IssueWorkflow.class); IssueUpdater updater = mock(IssueUpdater.class); ResourcePerspectives perspectives = mock(ResourcePerspectives.class); - Date loadedDate = new Date(); RulesProfile profile = mock(RulesProfile.class); RuleFinder ruleFinder = mock(RuleFinder.class); @Before public void init() { - when(initialOpenIssues.getLoadedDate()).thenReturn(loadedDate); decorator = new IssueTrackingDecorator( issueCache, initialOpenIssues, @@ -97,7 +98,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { // INPUT : one issue, no open issues during previous scan, no filtering when(issueCache.byComponent("struts:Action.java")).thenReturn(Arrays.asList(issue)); List dbIssues = Collections.emptyList(); - when(initialOpenIssues.selectAndRemove(123)).thenReturn(dbIssues); + when(initialOpenIssues.selectAndRemove("struts:Action.java")).thenReturn(dbIssues); decorator.doDecorate(file); @@ -205,7 +206,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { DefaultIssue openIssue = new DefaultIssue(); when(issueCache.byComponent("struts")).thenReturn(Arrays.asList(openIssue)); IssueDto deadIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle"); - when(initialOpenIssues.getAllIssues()).thenReturn(Arrays.asList(deadIssue)); + when(initialOpenIssues.selectAll()).thenReturn(Arrays.asList(deadIssue)); decorator.doDecorate(project); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java index a3a7ca31bac..7b93ecfb41f 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java @@ -22,6 +22,7 @@ package org.sonar.core.issue.db; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; +import org.apache.ibatis.session.ResultHandler; import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; @@ -61,11 +62,11 @@ public class IssueDao implements BatchComponent, ServerComponent { } } - public List selectNonClosedIssuesByModule(int componentId) { + public void selectNonClosedIssuesByModule(int componentId, ResultHandler handler) { SqlSession session = mybatis.openSession(); try { - IssueMapper mapper = session.getMapper(IssueMapper.class); - return mapper.selectNonClosedIssuesByModule(componentId); + session.select("org.sonar.core.issue.db.IssueMapper.selectNonClosedIssuesByModule", componentId, handler); + } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java index 84d6677376e..455c51b2201 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java @@ -30,12 +30,13 @@ import org.sonar.core.issue.DefaultIssue; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import java.io.Serializable; import java.util.Date; /** * @since 3.6 */ -public final class IssueDto { +public final class IssueDto implements Serializable { private Long id; private String kee; @@ -66,10 +67,10 @@ public final class IssueDto { private Date updatedAt; // joins - private transient String ruleKey; - private transient String ruleRepo; - private transient String componentKey; - private transient String rootComponentKey; + private String ruleKey; + private String ruleRepo; + private String componentKey; + private String rootComponentKey; public Long getId() { return id; diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java index 158824845ae..b5cbc0a65a7 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java @@ -22,6 +22,7 @@ package org.sonar.core.issue.db; import com.google.common.base.Function; import com.google.common.collect.Iterables; +import org.apache.ibatis.executor.result.DefaultResultHandler; import org.junit.Before; import org.junit.Test; import org.sonar.api.issue.IssueQuery; @@ -293,16 +294,19 @@ public class IssueDaoTest extends AbstractDaoTestCase { setupData("shared", "should_select_non_closed_issues_by_module"); // 400 is a non-root module - List dtos = dao.selectNonClosedIssuesByModule(400); - assertThat(dtos).hasSize(2); + DefaultResultHandler handler = new DefaultResultHandler(); + dao.selectNonClosedIssuesByModule(400, handler); + assertThat(handler.getResultList()).hasSize(2); - IssueDto issue = dtos.get(0); + IssueDto issue = (IssueDto) handler.getResultList().get(0); assertThat(issue.getRuleRepo()).isNotNull(); assertThat(issue.getRule()).isNotNull(); assertThat(issue.getComponentKey()).isNotNull(); // 399 is the root module. It does not have issues. - assertThat(dao.selectNonClosedIssuesByModule(399)).isEmpty(); + handler = new DefaultResultHandler(); + dao.selectNonClosedIssuesByModule(399, handler); + assertThat(handler.getResultList()).isEmpty(); } @@ -326,7 +330,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { } - private List getIssueIds(List issues){ + private List getIssueIds(List issues) { return newArrayList(Iterables.transform(issues, new Function() { @Override public Long apply(IssueDto input) { -- 2.39.5