From efc6109fa363997104e19a2cde253b97ff08eb6a Mon Sep 17 00:00:00 2001 From: Michal Maciejewski Date: Thu, 9 May 2019 10:42:09 +0200 Subject: [PATCH 1/6] speed up TourActivities.removeActivity() --- .../solution/route/activity/TourActivities.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java index 9730de76..fad9d186 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java @@ -164,10 +164,20 @@ public class TourActivities { for (TourActivity act : acts) { if (act == activity) { tourActivities.remove(act); + if (job == null || jobIsAlsoAssociateToOtherActs) { + // this is not a job activity OR other activities also refer to job --> do not remove job + // thus no need to iterate any further + return true; + } actRemoved = true; } else { - if (act instanceof JobActivity && job != null) { + if (job != null && act instanceof JobActivity) { if (((JobActivity) act).getJob().equals(job)) { + if (actRemoved) { + // other activities also refer to job --> do not remove job + // thus no need to iterate any further + return true; + } jobIsAlsoAssociateToOtherActs = true; } } From 4ac181165c8e22047db5ee47f83be1542e55c23b Mon Sep 17 00:00:00 2001 From: Michal Maciejewski Date: Thu, 9 May 2019 10:44:43 +0200 Subject: [PATCH 2/6] remove BreakScheduling.modifiedRoutes and its updating - it is never used --- .../jsprit/core/algorithm/recreate/BreakScheduling.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java index 66fbb182..a64b8154 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java @@ -45,8 +45,6 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe private final EventListeners eventListeners; - private Set modifiedRoutes = new HashSet(); - public BreakScheduling(VehicleRoutingProblem vrp, StateManager stateManager, ConstraintManager constraintManager) { this.stateManager = stateManager; this.breakInsertionCalculator = new BreakInsertionCalculator(vrp.getTransportCosts(), vrp.getActivityCosts(), new LocalActivityInsertionCostsCalculator(vrp.getTransportCosts(), vrp.getActivityCosts(), stateManager), constraintManager, vrp.getJobActivityFactory()); @@ -78,7 +76,6 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe @Override public void ruinStarts(Collection routes) { - } @Override @@ -88,7 +85,7 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe boolean removed = route.getTourActivities().removeJob(aBreak); if(removed) logger.trace("ruin: {}", aBreak.getId()); } - List breaks = new ArrayList(); + List breaks = new ArrayList<>(); for (Job j : unassignedJobs) { if (j instanceof Break) { breaks.add((Break) j); @@ -99,7 +96,6 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe @Override public void removed(Job job, VehicleRoute fromRoute) { - if(fromRoute.getVehicle().getBreak() != null) modifiedRoutes.add(fromRoute); } @Override @@ -119,6 +115,5 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe } } } - } } From dd122b54e65488929289494cd58ec9c5ddaf2b2f Mon Sep 17 00:00:00 2001 From: Michal Maciejewski Date: Thu, 9 May 2019 11:17:14 +0200 Subject: [PATCH 3/6] simplify TourActivities.removeActivity() by handling non-job activities separately --- .../route/activity/TourActivities.java | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java index fad9d186..9bf54d89 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java @@ -154,36 +154,35 @@ public class TourActivities { * @return true if activity has been removed, false otherwise */ public boolean removeActivity(TourActivity activity) { - Job job = null; - if (activity instanceof JobActivity) { - job = ((JobActivity) activity).getJob(); + if (!(activity instanceof JobActivity)) { + //assumes that an activity can be added only once to tourActivities + return tourActivities.remove(activity); } + + Job job = ((JobActivity) activity).getJob(); boolean jobIsAlsoAssociateToOtherActs = false; boolean actRemoved = false; - List acts = new ArrayList<>(tourActivities); - for (TourActivity act : acts) { + for (TourActivity act : new ArrayList<>(tourActivities)) { if (act == activity) { tourActivities.remove(act); - if (job == null || jobIsAlsoAssociateToOtherActs) { - // this is not a job activity OR other activities also refer to job --> do not remove job + if (jobIsAlsoAssociateToOtherActs) { + // other activities also refer to job --> do not remove job // thus no need to iterate any further return true; } actRemoved = true; } else { - if (job != null && act instanceof JobActivity) { - if (((JobActivity) act).getJob().equals(job)) { - if (actRemoved) { - // other activities also refer to job --> do not remove job - // thus no need to iterate any further - return true; - } - jobIsAlsoAssociateToOtherActs = true; + if (act instanceof JobActivity && ((JobActivity) act).getJob().equals(job)) { + if (actRemoved) { + // other activities also refer to job --> do not remove job + // thus no need to iterate any further + return true; } + jobIsAlsoAssociateToOtherActs = true; } } } - if (!jobIsAlsoAssociateToOtherActs && actRemoved) { + if (actRemoved) { jobs.remove(job); } return actRemoved; From 3c9c8aa397ec8af26713b4b8645695fc37410de6 Mon Sep 17 00:00:00 2001 From: Michal Maciejewski Date: Mon, 13 May 2019 12:48:43 +0200 Subject: [PATCH 4/6] enable removal of non-job activities via TourActivities.iterator() --- .../route/activity/TourActivities.java | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java index 9bf54d89..22b9db33 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java @@ -81,7 +81,6 @@ public class TourActivities { } public TourActivities() { - } public List getActivities() { @@ -89,7 +88,30 @@ public class TourActivities { } public Iterator iterator() { - return tourActivities.iterator(); + final Iterator iterator = tourActivities.iterator(); + return new Iterator() { + private TourActivity lastReturned = null; + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public TourActivity next() { + return lastReturned = iterator.next(); + } + + @Override + public void remove() { + if (lastReturned instanceof JobActivity) { + throw new IllegalStateException("Cannot remove JobActivities via iterator. " + + "Use TourActivities.removeActivity(), or alternatively, consider TourActivities.removeJob()"); + } else { + iterator.remove(); + } + } + }; } public boolean isEmpty() { From 1fc64502096f61247667bd3421949f827bf4eb3f Mon Sep 17 00:00:00 2001 From: Michal Maciejewski Date: Tue, 20 Aug 2019 12:59:50 +0200 Subject: [PATCH 5/6] add test for removing nonJobActivity from TourActivities --- .../route/activity/TestTourActivities.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java b/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java index 51fac31b..6604bf2e 100644 --- a/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java +++ b/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java @@ -22,6 +22,7 @@ import com.graphhopper.jsprit.core.problem.job.Service; import com.graphhopper.jsprit.core.problem.job.Shipment; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import static org.junit.Assert.*; @@ -118,6 +119,19 @@ public class TestTourActivities { assertEquals(0, tour.getActivities().size()); } + @Test + public void removingNonJobActivityShouldWork() { + TourActivity nonJobAct = Mockito.mock(TourActivity.class); + + tour.addActivity(nonJobAct); + assertTrue(tour.getActivities().contains(nonJobAct)); + + tour.removeActivity(nonJobAct); + + assertTrue(tour.isEmpty()); + assertFalse(tour.getActivities().contains(nonJobAct)); + } + @Test public void removingActivityShouldWork() { tour.addActivity(act); From 8076d463d77a9cc40b74842bc29a08f2c86260f8 Mon Sep 17 00:00:00 2001 From: Michal Maciejewski Date: Thu, 22 Aug 2019 15:45:06 +0200 Subject: [PATCH 6/6] use trusty dist for oraclejdk8 build on travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 373829e3..3d221ae1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ matrix: fast_finish: true include: - jdk: oraclejdk8 + dist: trusty # Java 9 needs to be manually installed/upgraded # see: https://github.com/travis-ci/travis-ci/issues/2968#issuecomment-149164058 - jdk: oraclejdk9