Changeset 17243 in josm


Ignore:
Timestamp:
2020-10-19T15:25:47+02:00 (5 weeks ago)
Author:
GerdP
Message:

see #19885: memory leak with "temporary" objects in validator and actions

  • replace ChangeCommand by ChangePropertyCommand to avoid cloning primitives
  • also move the call of super.endTest() to the end of the method endTest(), else reported timings make not much sense
Location:
trunk/src/org/openstreetmap/josm/data/validation/tests
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java

    r16445 r17243  
    1414import java.util.stream.Collectors;
    1515
    16 import org.openstreetmap.josm.command.ChangeCommand;
     16import org.openstreetmap.josm.command.ChangeMembersCommand;
    1717import org.openstreetmap.josm.command.Command;
    1818import org.openstreetmap.josm.command.DeleteCommand;
     
    202202    @Override
    203203    public void endTest() {
    204         super.endTest();
    205204        for (Set<OsmPrimitive> duplicated : relations.values()) {
    206205            if (duplicated.size() > 1) {
     
    223222        }
    224223        relationsNoKeys = null;
     224        super.endTest();
    225225    }
    226226
     
    246246    @Override
    247247    public Command fixError(TestError testError) {
    248         if (testError.getCode() == SAME_RELATION) return null;
     248        if (!isFixable(testError)) return null;
     249
    249250        Set<Relation> relFix = testError.primitives(Relation.class)
    250                 .filter(r -> !r.isDeleted())
     251                .filter(r -> !r.isDeleted() || r.getDataSet() == null || r.getDataSet().getPrimitiveById(r) == null)
    251252                .collect(Collectors.toSet());
    252253
     
    259260        Relation relationWithRelations = null;
    260261        Collection<Relation> relRef = null;
    261         for (Relation w : relFix) {
    262             Collection<Relation> rel = w.referrers(Relation.class).collect(Collectors.toList());
     262        for (Relation r : relFix) {
     263            Collection<Relation> rel = r.referrers(Relation.class).collect(Collectors.toList());
    263264            if (!rel.isEmpty()) {
    264265                if (relationWithRelations != null)
    265266                    throw new AssertionError("Cannot fix duplicate relations: More than one relation is member of another relation.");
    266                 relationWithRelations = w;
     267                relationWithRelations = r;
    267268                relRef = rel;
    268269            }
    269270            // Only one relation will be kept - the one with lowest positive ID, if such exist
    270271            // or one "at random" if no such exists. Rest of the relations will be deleted
    271             if (!w.isNew() && (idToKeep == 0 || w.getId() < idToKeep)) {
    272                 idToKeep = w.getId();
    273                 relationToKeep = w;
     272            if (!r.isNew() && (idToKeep == 0 || r.getId() < idToKeep)) {
     273                idToKeep = r.getId();
     274                relationToKeep = r;
    274275            }
    275276        }
     
    280281        if (relationWithRelations != null && relRef != null && relationToKeep != relationWithRelations) {
    281282            for (Relation rel : relRef) {
    282                 Relation newRel = new Relation(rel);
    283                 for (int i = 0; i < newRel.getMembers().size(); ++i) {
    284                     RelationMember m = newRel.getMember(i);
     283                List<RelationMember> members = new ArrayList<>(rel.getMembers());
     284                for (int i = 0; i < rel.getMembers().size(); ++i) {
     285                    RelationMember m = rel.getMember(i);
    285286                    if (relationWithRelations.equals(m.getMember())) {
    286                         newRel.setMember(i, new RelationMember(m.getRole(), relationToKeep));
     287                        members.set(i, new RelationMember(m.getRole(), relationToKeep));
    287288                    }
    288289                }
    289                 commands.add(new ChangeCommand(rel, newRel));
     290                commands.add(new ChangeMembersCommand(rel, members));
    290291            }
    291292        }
  • trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java

    r16445 r17243  
    1616import java.util.stream.IntStream;
    1717
    18 import org.openstreetmap.josm.command.ChangeCommand;
     18import org.openstreetmap.josm.command.ChangeMembersCommand;
    1919import org.openstreetmap.josm.command.Command;
    2020import org.openstreetmap.josm.command.DeleteCommand;
     
    283283        if (wayWithRelations != null && relations != null && wayToKeep != wayWithRelations) {
    284284            for (Relation rel : relations) {
    285                 Relation newRel = new Relation(rel);
    286                 for (int i = 0; i < newRel.getMembers().size(); ++i) {
    287                     RelationMember m = newRel.getMember(i);
     285                List<RelationMember> members = rel.getMembers();
     286                for (int i = 0; i < rel.getMembers().size(); ++i) {
     287                    RelationMember m = rel.getMember(i);
    288288                    if (wayWithRelations.equals(m.getMember())) {
    289                         newRel.setMember(i, new RelationMember(m.getRole(), wayToKeep));
     289                        members.set(i, new RelationMember(m.getRole(), wayToKeep));
    290290                    }
    291291                }
    292                 commands.add(new ChangeCommand(rel, newRel));
     292                commands.add(new ChangeMembersCommand(rel, members));
    293293            }
    294294        }
  • trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java

    r16793 r17243  
    1818import java.util.stream.Collectors;
    1919
    20 import org.openstreetmap.josm.command.ChangeCommand;
     20import org.openstreetmap.josm.command.ChangeMembersCommand;
    2121import org.openstreetmap.josm.command.Command;
    2222import org.openstreetmap.josm.command.DeleteCommand;
     
    386386            if (testError.getCode() == RELATION_LOOP) {
    387387                Relation old = (Relation) primitives.iterator().next();
    388                 Relation mod = new Relation(old);
    389                 mod.removeMembersFor(primitives);
    390                 return new ChangeCommand(old, mod);
     388                List<RelationMember> remaining = new ArrayList<>(old.getMembers());
     389                remaining.removeIf(rm -> primitives.contains(rm.getMember()));
     390                return new ChangeMembersCommand(old, Utils.toUnmodifiableList(remaining));
    391391            }
    392392        }
Note: See TracChangeset for help on using the changeset viewer.