From 9cd05a88de9eddbd465ece540760799bb6512953 Mon Sep 17 00:00:00 2001 From: Jose Duarte Date: Wed, 26 Aug 2026 22:15:06 -0700 Subject: [PATCH 1/4] Avoiding sqrt when unnecessary. About 8% performance gain --- .../biojava/nbio/structure/contact/Grid.java | 11 ++++++ .../nbio/structure/contact/GridCell.java | 34 +++++++++++-------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java index 0047385ab2..a5e86fe568 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java @@ -66,6 +66,7 @@ public class Grid { private GridCell[][][] cells; private double cutoff; + private double cutoffSq; private int cellSize; private Point3d[] iAtoms; @@ -91,6 +92,7 @@ public class Grid { */ public Grid(double cutoff) { this.cutoff = cutoff; + this.cutoffSq = cutoff * cutoff; this.cellSize = (int) Math.floor(cutoff*SCALE); this.noOverlap = false; } @@ -496,6 +498,15 @@ public double getCutoff() { return cutoff; } + /** + * Returns the square of the cutoff, precomputed at construction. Used by {@link GridCell} to + * compare squared distances, avoiding a square root per candidate pair. + * @return the squared cutoff + */ + protected double getCutoffSq() { + return cutoffSq; + } + /** * Tells whether (after having added atoms to grid) the i and j grids are not overlapping. * Overlap is defined as enclosing bounds of the 2 grids being no more than one cell size apart. diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java index 6028110eb6..e0802f392a 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java @@ -74,14 +74,16 @@ public List getContactsWithinCell(){ Point3d[] iAtoms = grid.getIAtoms(); Point3d[] jAtoms = grid.getJAtoms(); - double cutoff = grid.getCutoff(); + // we compare squared distances to the squared cutoff, so that the expensive square root is + // only computed for the pairs that are actually in contact (the large majority are not) + double cutoffSq = grid.getCutoffSq(); if (jAtoms==null) { for (int i:iIndices) { for (int j:iIndices) { if (j>i) { - double distance = iAtoms[i].distance(iAtoms[j]); - if (distance getContactsWithinCell(){ } else { for (int i:iIndices) { for (int j:jIndices) { - double distance = iAtoms[i].distance(jAtoms[j]); - if (distance getContactsToOtherCell(GridCell otherCell){ Point3d[] iAtoms = grid.getIAtoms(); Point3d[] jAtoms = grid.getJAtoms(); - double cutoff = grid.getCutoff(); + // we compare squared distances to the squared cutoff, so that the expensive square root is + // only computed for the pairs that are actually in contact (the large majority are not) + double cutoffSq = grid.getCutoffSq(); if (jAtoms==null) { @@ -119,8 +123,8 @@ public List getContactsToOtherCell(GridCell otherCell){ for (int i:iIndices) { for (int j:otherCell.iIndices) { if (j>i) { - double distance = iAtoms[i].distance(iAtoms[j]); - if (distance getContactsToOtherCell(GridCell otherCell){ for (int i:iIndices) { for (int j:otherCell.jIndices) { - double distance = iAtoms[i].distance(jAtoms[j]); - if (distance getContactsToOtherCell(GridCell otherCell){ * @return */ public boolean hasContactToAtom(Point3d[] iAtoms, Point3d[] jAtoms, Point3d query, double cutoff) { + // only the comparison matters here, so we can stay in squared distance space and avoid square roots altogether + double cutoffSq = cutoff * cutoff; for( int i : iIndices ) { - double distance = iAtoms[i].distance(query); - if( distance Date: Wed, 26 Aug 2026 22:35:26 -0700 Subject: [PATCH 2/4] Use int arrays for performance. Gain is ~ 1.5x --- .../nbio/structure/contact/GridCell.java | 98 ++++++++++++++----- 1 file changed, 73 insertions(+), 25 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java index e0802f392a..9c75fa1c75 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/GridCell.java @@ -21,6 +21,7 @@ package org.biojava.nbio.structure.contact; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.vecmath.Point3d; @@ -35,30 +36,61 @@ public class GridCell { + /** + * Shared empty array so that cells that never receive indices (e.g. the j indices when only one + * set of atoms was added to the grid) don't allocate anything at all. + */ + private static final int[] EMPTY = new int[0]; + + /** + * Capacity of the index arrays on first insertion. Cell occupancy depends on the cutoff (the cell + * side is the cutoff), ranging from a handful of atoms for small cutoffs to a few tens for large + * ones, so we start small and grow geometrically. + */ + private static final int INITIAL_CAPACITY = 8; + private Grid grid; - private ArrayList iIndices; - private ArrayList jIndices; + + /** + * The indices of the i atoms in this cell, held as a primitive array to avoid the boxing (and the + * pointer chasing it entails) of a Collection of Integers: these are read in the innermost loop of + * the contact calculation. Only the first {@link #numIindices} elements are meaningful. + */ + private int[] iIndices; + private int numIindices; + + /** + * The indices of the j atoms in this cell. See {@link #iIndices}. + */ + private int[] jIndices; + private int numJindices; public GridCell(Grid parent){ - iIndices = new ArrayList<>(); - jIndices = new ArrayList<>(); + iIndices = EMPTY; + jIndices = EMPTY; this.grid = parent; } public void addIindex(int serial){ - iIndices.add(serial); + if (numIindices == iIndices.length) { + iIndices = Arrays.copyOf(iIndices, numIindices == 0 ? INITIAL_CAPACITY : numIindices * 2); + } + iIndices[numIindices++] = serial; } public void addJindex(int serial){ - jIndices.add(serial); + if (numJindices == jIndices.length) { + jIndices = Arrays.copyOf(jIndices, numJindices == 0 ? INITIAL_CAPACITY : numJindices * 2); + } + jIndices[numJindices++] = serial; } public int getNumIindices() { - return iIndices.size(); + return numIindices; } public int getNumJindices() { - return jIndices.size(); + return numJindices; } /** @@ -79,19 +111,25 @@ public List getContactsWithinCell(){ double cutoffSq = grid.getCutoffSq(); if (jAtoms==null) { - for (int i:iIndices) { - for (int j:iIndices) { + for (int a=0; ai) { - double distanceSq = iAtoms[i].distanceSquared(iAtoms[j]); + double distanceSq = atomI.distanceSquared(iAtoms[j]); if (distanceSq getContactsToOtherCell(GridCell otherCell){ if (jAtoms==null) { - for (int i:iIndices) { - for (int j:otherCell.iIndices) { + int[] otherIndices = otherCell.iIndices; + int otherNum = otherCell.numIindices; + for (int a=0; ai) { - double distanceSq = iAtoms[i].distanceSquared(iAtoms[j]); + double distanceSq = atomI.distanceSquared(iAtoms[j]); if (distanceSq getContactsToOtherCell(GridCell otherCell){ } else { - for (int i:iIndices) { - for (int j:otherCell.jIndices) { - double distanceSq = iAtoms[i].distanceSquared(jAtoms[j]); + int[] otherIndices = otherCell.jIndices; + int otherNum = otherCell.numJindices; + for (int a=0; a getContactsToOtherCell(GridCell otherCell){ public boolean hasContactToAtom(Point3d[] iAtoms, Point3d[] jAtoms, Point3d query, double cutoff) { // only the comparison matters here, so we can stay in squared distance space and avoid square roots altogether double cutoffSq = cutoff * cutoff; - for( int i : iIndices ) { - double distanceSq = iAtoms[i].distanceSquared(query); + for (int a=0; a Date: Thu, 27 Aug 2026 21:21:11 -0700 Subject: [PATCH 3/4] Presize the AtomContactSet HashMap. Performance gain is ~ 1.2x --- .../structure/contact/AtomContactSet.java | 20 +++++++++++++++++++ .../biojava/nbio/structure/contact/Grid.java | 6 ++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java index 34de552786..a4757de486 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java @@ -37,6 +37,12 @@ public class AtomContactSet implements Serializable, Iterable { private static final long serialVersionUID = 1L; + /** + * The default load factor of a {@link HashMap}, needed to size the map from an expected number of + * entries. + */ + private static final float DEFAULT_LOAD_FACTOR = 0.75f; + private HashMap, AtomContact> contacts; private double cutoff; @@ -45,6 +51,20 @@ public AtomContactSet(double cutoff) { this.contacts = new HashMap<>(); } + /** + * Creates an AtomContactSet sized to hold the given number of contacts, so that the underlying map + * doesn't have to be repeatedly resized and rehashed as contacts are added. Contact calculations + * produce hundreds of thousands of contacts for a large structure, where the repeated rehashing + * that growing from the default capacity entails is a significant part of the cost. + * @param cutoff the distance cutoff + * @param expectedSize the number of contacts expected to be added. Only affects performance: an + * over-estimate merely leaves the map larger than it needs to be. + */ + public AtomContactSet(double cutoff, int expectedSize) { + this.cutoff = cutoff; + this.contacts = new HashMap<>((int) (expectedSize / DEFAULT_LOAD_FACTOR) + 1); + } + public void add(AtomContact contact) { this.contacts.put(getAtomIdPairFromContact(contact), contact); } diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java index a5e86fe568..ba2fd08cdb 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/Grid.java @@ -382,10 +382,12 @@ private int[] getIntBounds(BoundingBox coordbounds) { */ public AtomContactSet getAtomContacts() { - AtomContactSet contacts = new AtomContactSet(cutoff); - List list = getIndicesContacts(); + // each contact maps to at most one entry in the set, so the number of index contacts sizes it + // without ever under-allocating + AtomContactSet contacts = new AtomContactSet(cutoff, list.size()); + if (jAtomObjects == null) { for (Contact cont : list) { contacts.add(new AtomContact(new Pair(iAtomObjects[cont.getI()],iAtomObjects[cont.getJ()]),cont.getDistance())); From e73ce56fb1fd9f17cc8238f9d38b420445e4ef93 Mon Sep 17 00:00:00 2001 From: Jose Duarte Date: Wed, 2 Sep 2026 21:07:13 -0700 Subject: [PATCH 4/4] Docs --- .../structure/contact/AtomContactSet.java | 58 +++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java index a4757de486..9748065524 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/contact/AtomContactSet.java @@ -28,6 +28,22 @@ /** * A set of atom-atom contacts to hold the results of intra and inter-chain contact calculations + *

+ * Contacts are keyed by the ordered pair of {@link AtomIdentifier}s of the 2 atoms, i.e. the + * pair (a,b) and the pair (b,a) are 2 different keys. Thus look-ups ({@link #hasContact(Atom, Atom)}, + * {@link #getContact(Atom, Atom)}) must give the 2 atoms in the same order in which the contact was + * calculated. The order produced by the calculation ({@link Grid}) is: + *

    + *
  • for contacts within a single set of atoms (e.g. the intra-chain contacts of + * StructureTools.getAtomsInContact(Chain, double)), the atom that comes first in the + * atom array is the first member of the pair
    • + *
  • for contacts between 2 sets of atoms (e.g. the inter-chain contacts of + * StructureTools.getAtomsInContact(Chain, Chain, double, boolean), or a + * {@link StructureInterface}), the atom belonging to the first set is the first member of the pair
    • + *
+ *

+ * Note that the order is not the order of PDB serials or of any other property of the atoms + * themselves: it is only the order in which the atoms were given to the calculation. * * @author duarte_j * @@ -65,31 +81,65 @@ public AtomContactSet(double cutoff, int expectedSize) { this.contacts = new HashMap<>((int) (expectedSize / DEFAULT_LOAD_FACTOR) + 1); } + /** + * Adds the given contact to this set, keyed by the ordered pair of its 2 atoms. If a contact + * with the same ordered pair of atoms is already present it is replaced. + * @param contact the contact to add + */ public void add(AtomContact contact) { this.contacts.put(getAtomIdPairFromContact(contact), contact); } + /** + * Adds all given contacts to this set, see {@link #add(AtomContact)}. + * @param list the contacts to add + */ public void addAll(Collection list) { for (AtomContact contact:list) { this.contacts.put(getAtomIdPairFromContact(contact), contact); } } + /** + * Tells whether a contact exists between the 2 given atoms, in the given order. + *

+ * The 2 atoms have to be passed in the same order in which the contacts of this set were + * calculated, otherwise this returns false even if the 2 atoms are within the distance cutoff. + * See the class documentation for the ordering convention. If the order is not known, both orders + * have to be queried. + * @param atom1 the first atom of the pair + * @param atom2 the second atom of the pair + * @return true if the 2 atoms are in contact in the given order, false otherwise + * @see #getContact(Atom, Atom) + */ public boolean hasContact(Atom atom1, Atom atom2) { return hasContact( new AtomIdentifier(atom1.getPDBserial(),atom1.getGroup().getChainId()), new AtomIdentifier(atom2.getPDBserial(),atom2.getGroup().getChainId()) ); } + /** + * Tells whether a contact exists between the 2 given atom identifiers, in the given order, + * see {@link #hasContact(Atom, Atom)}. + * @param atomId1 the identifier of the first atom of the pair + * @param atomId2 the identifier of the second atom of the pair + * @return true if the 2 atoms are in contact in the given order, false otherwise + */ public boolean hasContact(AtomIdentifier atomId1, AtomIdentifier atomId2) { return contacts.containsKey(new Pair(atomId1,atomId2)); } /** - * Returns the corresponding AtomContact or null if no contact exists between the 2 given atoms - * @param atom1 - * @param atom2 - * @return + * Returns the contact between the 2 given atoms in the given order, or null if there is + * no such contact in this set. + *

+ * As in {@link #hasContact(Atom, Atom)} the order of the 2 atoms matters: they have to be passed + * in the same order in which the contacts of this set were calculated, otherwise null is returned + * even if the 2 atoms are within the distance cutoff. See the class documentation for the + * ordering convention. + * @param atom1 the first atom of the pair + * @param atom2 the second atom of the pair + * @return the contact between the 2 atoms in the given order, or null if there is none */ public AtomContact getContact(Atom atom1, Atom atom2) { return contacts.get(new Pair(