535879: [performance] Loading a large tasklist.xml takes a long time
Changed the order the tasks are added to the task list to prevent
unnecessary removals from UnmatchedTaskContainer when loading
tasklist.xml.
Change-Id: I1931578f3c9cdd3e47fc5f2324577ec83874fc27
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=535879
Signed-off-by: alexei.trebounskikh <alexei.trebounskikh@tasktop.com>
(cherry picked from commit 3e12a215f3e68d8cf11ccf84196afb42178f2cfa)
diff --git a/org.eclipse.mylyn.tasks.core.tests/src/org/eclipse/mylyn/internal/tasks/core/externalization/LazyTransferListTest.java b/org.eclipse.mylyn.tasks.core.tests/src/org/eclipse/mylyn/internal/tasks/core/externalization/LazyTransferListTest.java
new file mode 100644
index 0000000..c66e677
--- /dev/null
+++ b/org.eclipse.mylyn.tasks.core.tests/src/org/eclipse/mylyn/internal/tasks/core/externalization/LazyTransferListTest.java
@@ -0,0 +1,81 @@
+/*******************************************************************************
+ * Copyright (c) 2018 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.internal.tasks.core.externalization;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import org.eclipse.mylyn.internal.tasks.core.AbstractTask;
+import org.eclipse.mylyn.internal.tasks.core.ITransferList;
+import org.eclipse.mylyn.internal.tasks.core.TaskList;
+import org.eclipse.mylyn.internal.tasks.core.TaskTask;
+import org.junit.Before;
+import org.junit.Test;
+
+public class LazyTransferListTest {
+
+ private ITransferList taskList;
+
+ private LazyTransferList lazyList;
+
+ @Before
+ public void setUp() throws Exception {
+ taskList = mock(TaskList.class);
+ lazyList = new LazyTransferList(taskList);
+ }
+
+ @Test
+ public void addUnmatchedTask() {
+ AbstractTask task = spy(new TaskTask("kind", "repoUrl", "id"));
+
+ // add to unmatched
+ lazyList.addTask(task);
+
+ // should not add task to task list
+ verify(taskList, never()).addTask(task);
+ verify(taskList, never()).addTask(eq(task), any());
+
+ // commit should add the task to task list
+ lazyList.commit();
+ verify(taskList).addTask(task);
+
+ // subsequent commit should not add task again
+ lazyList.commit();
+ verifyNoMoreInteractions(taskList);
+ }
+
+ @Test
+ public void addSubTask() {
+ AbstractTask task = spy(new TaskTask("kind", "repoUrl", "parent"));
+ AbstractTask subTask = spy(new TaskTask("kind", "repoUrl", "child"));
+
+ // add subtask to a task container
+ lazyList.addTask(subTask);
+ lazyList.addTask(task);
+ lazyList.addTask(subTask, task);
+
+ // should add task and subtask to task list
+ verify(taskList).addTask(task);
+ verify(taskList).addTask(subTask, task);
+ verifyNoMoreInteractions(taskList);
+
+ // commit should not add anything to task list
+ lazyList.commit();
+ verifyNoMoreInteractions(taskList);
+ }
+
+}
diff --git a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/LazyTransferList.java b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/LazyTransferList.java
new file mode 100644
index 0000000..a1a9604
--- /dev/null
+++ b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/LazyTransferList.java
@@ -0,0 +1,130 @@
+/*******************************************************************************
+ * Copyright (c) 2018 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.internal.tasks.core.externalization;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.eclipse.mylyn.internal.tasks.core.AbstractTask;
+import org.eclipse.mylyn.internal.tasks.core.AbstractTaskCategory;
+import org.eclipse.mylyn.internal.tasks.core.AbstractTaskContainer;
+import org.eclipse.mylyn.internal.tasks.core.ITransferList;
+import org.eclipse.mylyn.internal.tasks.core.RepositoryQuery;
+import org.eclipse.mylyn.internal.tasks.core.TaskCategory;
+import org.eclipse.mylyn.internal.tasks.core.UnmatchedTaskContainer;
+import org.eclipse.mylyn.tasks.core.ITask;
+
+/**
+ * This class delegates the calls to underlying {@link ITransferList} while altering behavior of {@link #addTask(ITask)}
+ * and {@link #addTask(ITask, category)} methods to avoid adding tasks being loaded to task list's
+ * {@link UnmatchedTaskContainer} if they belong to a different task container. Instead, tasks added with
+ * {@link #addTask(ITask)} method are stored internally, and only pushed to underlying {@link ITransferList} during
+ * subsequent calls to {@link #addTask(ITask, category)} or on final {@link #commit(void)}.
+ * <p>
+ * <b>Usage</b>
+ * <p>
+ * The caller can call {@link #addTask(ITask)} and {@link #addTask(ITask, category)} methods to load task list as usual.
+ * <p>
+ * The caller is expected to call {@link #commit(void)} to add the remaining uncategorized tasks to task list.
+ *
+ * @see {@link #addTask(ITask)}
+ * @see {@link #addTask(ITask, AbstractTaskContainer)}
+ * @see {@link #commit()}
+ */
+class LazyTransferList implements ITransferList {
+
+ private final Map<String, ITask> untransferedTasks = new HashMap<>();
+
+ private final ITransferList taskList;
+
+ LazyTransferList(ITransferList taskList) {
+ this.taskList = taskList;
+ }
+
+ public void addCategory(TaskCategory category) {
+ taskList.addCategory(category);
+ }
+
+ public void addQuery(RepositoryQuery query) {
+ taskList.addQuery(query);
+ }
+
+ public void addTask(ITask task) {
+ untransferedTasks.put(task.getHandleIdentifier(), task);
+ }
+
+ public boolean addTask(ITask task, AbstractTaskContainer parentContainer) {
+ boolean result = taskList.addTask(task, commit(parentContainer));
+ untransferedTasks.remove(task.getHandleIdentifier());
+ return result;
+ }
+
+ public AbstractTaskCategory getContainerForHandle(String handle) {
+ return taskList.getContainerForHandle(handle);
+ }
+
+ public Collection<AbstractTask> getAllTasks() {
+ return taskList.getAllTasks();
+ }
+
+ public Set<AbstractTaskCategory> getCategories() {
+ return taskList.getCategories();
+ }
+
+ public Set<RepositoryQuery> getQueries() {
+ return taskList.getQueries();
+ }
+
+ public AbstractTask getTask(String handleIdentifier) {
+ AbstractTask task = (AbstractTask) untransferedTasks.get(handleIdentifier);
+
+ if (task == null) {
+ task = taskList.getTask(handleIdentifier);
+ }
+
+ return task;
+ }
+
+ public ITask getTask(String repositoryUrl, String taskId) {
+ return taskList.getTask(repositoryUrl, taskId);
+ }
+
+ /**
+ * If the container is an {@link ITask}, pushes it to task list
+ *
+ * @param container
+ */
+ private AbstractTaskContainer commit(AbstractTaskContainer container) {
+ if (container instanceof ITask) {
+ AbstractTask task = (AbstractTask) untransferedTasks.get(container.getHandleIdentifier());
+
+ if (task != null) {
+ taskList.addTask(task);
+ untransferedTasks.remove(container.getHandleIdentifier());
+ }
+ }
+
+ return container;
+ }
+
+ /**
+ * Pushes the remaining tasks to task list
+ */
+ public void commit() {
+ for (ITask task : untransferedTasks.values()) {
+ taskList.addTask(task);
+ }
+ untransferedTasks.clear();
+ }
+}
\ No newline at end of file
diff --git a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/SaxTaskListHandler.java b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/SaxTaskListHandler.java
index 394d02e..7a1e38b 100644
--- a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/SaxTaskListHandler.java
+++ b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/externalization/SaxTaskListHandler.java
@@ -34,7 +34,7 @@
public class SaxTaskListHandler extends DefaultHandler {
- private final ITransferList taskList;
+ private final LazyTransferList taskList;
private final RepositoryModel repositoryModel;
@@ -52,7 +52,7 @@
public SaxTaskListHandler(ITransferList taskList, RepositoryModel repositoryModel,
IRepositoryManager repositoryManager) throws CoreException {
- this.taskList = taskList;
+ this.taskList = new LazyTransferList(taskList);
this.repositoryModel = repositoryModel;
this.repositoryManager = repositoryManager;
@@ -151,7 +151,7 @@
applyContainmentToTaskList(subTasks);
applyContainmentToTaskList(queryResults);
applyContainmentToTaskList(categorizedTasks);
-
+ commitUntransferedTasksToTaskList();
break;
default:
break;
@@ -161,6 +161,10 @@
}
+ private void commitUntransferedTasksToTaskList() {
+ taskList.commit();
+ }
+
private <T extends IRepositoryElement> void recordHit(Attributes attributes, Multimap<T, String> hitMap,
SaxTaskListElementBuilder<T> builder) {
String handle = attributes.getValue(TaskListExternalizationConstants.KEY_HANDLE);
