Skip to content

Collision Performance#4

Open
AmityWilder wants to merge 9 commits intoanadon:masterfrom
AmityWilder:collision-perf
Open

Collision Performance#4
AmityWilder wants to merge 9 commits intoanadon:masterfrom
AmityWilder:collision-perf

Conversation

@AmityWilder
Copy link

@AmityWilder AmityWilder commented Feb 8, 2026

  • Copy element Sets to temporary arrays and ArrayLists, since they will be iterated over far more than they will be searched within the overlap method.
  • Implement a bounding box for the selection and remove any elements not overlapping the bounding box from the temporary unselected element ArrayList before any further checks.
    • Use swap-removal since order doesn't matter, making the removal worst case $O(1)$ instead of $O(n)$
  • Crunch the $O(n^3)$ section in the "else" case for simple intersections down to $O(n^2)$, since that part seems to just be checking against all elements per element per selected element--despite only involving the selected element and all other elements, not the selected element and all permutations of other elements.
    i.e.
for (Element sel : selected) {          // O(n)
  for (Element el : elementsArr) {      // O(n^2)
    if (intersecting) { ... }
    else {
      for (Element elm : elementsArr) { // O(n^3)
        if (sel ? elm) // no mention of `el`

$$\large\Downarrow$$

for (Element sel : selected) {       // O(n)
  for (Element el : elementsArr) {   // O(n^2)
    if (intersecting) { ... }
  }
  if (none intersected) {
    for (Element el : elementsArr) { // O(n^2)
      if (sel ? el) // ...

Related to #3

Collections was imported for swap remove, before realizing swapping wasnt needed and copying would suffice.
…loop

I'm not sure that "for (sel : selected) { for (el : elements) { for (elm : elements) { /* only acknowledging sel and elm*/ } } }" makes sense. Perhaps at some point in development, the "sel.intersects(el)" may have been mistaken for a bulk test of all the elements?
The way it was written prior to this commit, it would check all elements for all elements, turning an O(n^2) operation into an O(n^3) operation with n:1 duplicate operations (if I am understanding the intent of the code correctly).
@AmityWilder
Copy link
Author

Apologies for my IDE removing all the trailing whitespace in the files I edited, I didn't notice it had done that until creating this PR.

Also, feel free to remove (or ask me to remove, if that's easier) the edits to the README.md; I understand that may be out of scope for this.

@anadon
Copy link
Owner

anadon commented Feb 8, 2026 via email

@AmityWilder
Copy link
Author

AmityWilder commented Feb 9, 2026

My Computer Organization and Algorithm Engineering professor, Dr. Lalejini, put me in contact with Dr. Kurmas over email after I offered to look into the collision performance. JLS was running rather slowly when I tried building a 16-bit adder without using subcircuits.

I promise I did not use AI (I personally hate the stuff and 100% understand your apprehension), but I don't exactly have a 5hr recording of myself writing the code. How would you like me to prove this wasn't AI generated? I can explain every change and every step of my process in depth if that would work.

@anadon
Copy link
Owner

anadon commented Feb 9, 2026 via email

@bsiever
Copy link

bsiever commented Feb 10, 2026

@anadon FWIW, I created a fork of your work, made some updates, and continue to use JLS in classes (https://github.com/bsiever/JLS/) --- your work definitely wasn't for naught!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants