Conversation
… "removeAll(collection)"
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).
|
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 |
|
This is an OLD project initially created by Dr. Poplawski at Michigan
Technological University. I'll need to ask you for proof that this
submission isn't AI and your interest in the project. More contemporary
and industry accepted work builds on Verilog or SystemC.
…On Sun, Feb 8, 2026, 4:42 PM Amy Wilder ***@***.***> wrote:
*AmityWilder* left a comment (anadon/JLS#4)
<#4 (comment)>
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.
—
Reply to this email directly, view it on GitHub
<#4 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXYEBICEGK7LQNAHUCGWDL4K6UUFAVCNFSM6AAAAACUNDL64GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNRYGM2TCNJZHE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
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. |
|
Saying so is good enough for me. Glad to know my archival work of this
program wasn't for naught. I'll get to reviewing the PR tonight.
…On Mon, Feb 9, 2026 at 10:09 AM Amy Wilder ***@***.***> wrote:
*AmityWilder* left a comment (anadon/JLS#4)
<#4 (comment)>
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.
—
Reply to this email directly, view it on GitHub
<#4 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXYEBMYGIRKHJQGNWDCCW34LCPMJAVCNFSM6AAAAACUNDL64GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNZSGE3DGMJUHE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
@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! |
| int i = 0; | ||
| while (i < elementsArr.size()) { | ||
| if (selected.contains(elementsArr.get(i))) { | ||
| // "swap" remove - slightly cheaper because we don't care about order |
There was a problem hiding this comment.
If you're very worried about performance for this operation, a linked list has better cache locality.
There was a problem hiding this comment.
I'm a bit confused how that can be true...
An array has all of its elements next to each other in memory, so the entire surrounding chunk of memory can be pulled into memory at once and contain those adjacent elements.
A linked list has pointers to adjacent elements, meaning it couldn't be cached without dereferencing those pointers--which in addition to being extra operations (and slow ones at that: loading data from RAM) may be unsafe to do speculatively since the CPU can't be certain those pointers belong to the program, or are even valid memory addresses.
Am I incorrect?
There was a problem hiding this comment.
You'd be almost right if this were C. Less so for Java. In a traditional linked list and doubly linked list, each node has one to three pointers. One is for the next node, optionally for the previous node, and the value is either embedded or has a pointer to it. However, in optimized implementations these nodes are kept densely packed in an array or something more exotic. So, you're comparing something called a long jump to overcome 2+GB changes in virtual address or physical address (we're skipping the kernel considerations for now) or if correctly optimized in some circumstances a standard jump. Those jumps are, practically, cache-busters. Intel has some fancy stuff with their pre-fetcher, but relying on that from a programmer's perspective is faulty at best, meaning that you count on missing L1, L2, L3 (and sometimes L4) cache, hitting RAM, and killing the performance. Now, when working with swapping to the end of a dense array, you're busting this cache on every swap just by virtue of touching the end of the array while you were just working on the beginning. However, with Java's presumably optimized implementation, these removals are always touching bytes which are within one or two words away. This means that the cache characteristics are great. So even if you're performing 5X the operations, the fact this the CPU assumes that you're working with memory right next to each other allows for simpler algorithms to pre-fetch that memory and avoids the cache misses in the first place. And this is without the extremely optimized data structures which are actually probably being used which are going to have better characteristics. For these reasons, trust the standard library. Neither of us are going to out-smart it.
Once you hit system organization, operating systems, and compilers you'll formally be introduced to these topics if your profs are good.
One tip for compilers -- learn how to use stack machines. They aren't always covered but they allow you to entirely skip register management and it makes your life immensely easier.
There was a problem hiding this comment.
I see! I had completely not realized that accessing the end would be what causes the slowdown (and I had NO idea about any of those linked list optimizations), but it makes perfect sense after you explained it!
I'll try benchmarking this and an alternative linked list implementation and keep whichever is faster. However, I'm not entirely sure how to benchmark this code without its speed being influenced by the speed of my own movements. As far as I can tell, there isn't a good way of isolating the overlap code and running it with a variety specific circuits--I would have to trigger the overlap method manually. Do you have any suggestions?
| // so it makes sense to use a temporary array for cache locality and O(n) traversal. | ||
| Element[] selectedArr = selected.toArray(Element[]::new); | ||
| ArrayList<Element> elementsArr = new ArrayList(circuit.getElements()); | ||
| // elementsArr.removeAll(selected); |
There was a problem hiding this comment.
These top level functions often have significant hidden optimizations to them. I'd benchmark before doing this and claiming this is the part that improves performance.
src/jls/edit/SimpleEditor.java
Outdated
| int ymax = Integer.MIN_VALUE; | ||
|
|
||
| // build up a bounding box | ||
| for (Element sel : selectedArr) { |
There was a problem hiding this comment.
This is incorrect for concave geometries.
There was a problem hiding this comment.
Further on this point, say a student wants to work on a component involving several parts of a CPU pipeline in a ring-like layout on the diagram, and then they want to re-join in to the full CPU area. It is most likely that the lines do validly connect. But with this, the rectangularized area of each will indicate a false positive for overlapping and reject the drop. Rather than this approach, you should be looking at partitioning algorithms with non-zero area elements. You're likely to find something which generates a layout in O(n*log(n)) time. If you then couple this with a de-bouncer and a early termination signal for a separate thread, you'll freely be able to move the selected elements and only allow placement after the thread for the current position terminates and indicates success.
There was a problem hiding this comment.
I have encountered such a false-negative on this branch myself, so I can confirm with 100% certainty that you are correct.
For now, I have removed the bounding box part of the code in 9275399. I can try working on your suggested alternative to a bounding box once I'm sure everything else is correct.
There was a problem hiding this comment.
I apologize, you were talking about a false positive. I encountered a false negative and mistook it for what you were saying.
The false negative I encountered was that, when selecting wires, the user could place them in such a way that they overlap a wire or pin that does not overlap the selection bounding box.
Either way, I believe the bounding box check may be a bust and/or need work.
|
Thinking further, I think the move-and-drop implementation should take advantage that elements and voxels in 2D space and have their collisions detected by generating a sorted list for each of non-moving elements' X-values, moving elements' X-values, non-moving elements' Y-values, and moving elements' Y-values. Use counting sort. Have two threads with kill logic handle each of the X and Y dimensions. Use a transform for the granularized cursor coordinates to detect movements and launch threads with correct parameters. If in the line-segments for the elements in each merged array have a line-segment overlap for the same element, there is a collision. Segment collision detection is trivial. These sorted lists will need to be sorted on keys with their coordinate integer value, and a value of a list containing references to element objects. This should yield O(n) performance. |
|
@AmityWilder Are you about? |
|
Sorry, I've been getting pulled in a lot of directions lately and I wanted to research your response before giving a yes or no. |
|
My immediate thought is whether two I imagine, given the way students are encouraged to prefer smaller circuits containing subcircuits, most might not exceed a value of Plus, the bigger part from my perspective (as the person who will possibly be implementing this): Your suggestion for changing it from I'm not saying no, I just want to make sure it's worth it before spending what could be an entire week on what could very well amount to a minimal, possibly even negative performance change. That's why I want to give it some extra thought before jumping headfirst into it. It just happens that I'm having to wait for the right time to research it, as at this moment it's approching the 7th week of this 14-week semester, I have 5 classes, I've been doing bug fixes on Bill Siever's repo, and Dr. Kurmas has suggested some additional projects for me. I will absolutely look into this when I have the chance. I apologize for not answering sooner, your previous reply was automatically marked as read when I read it but I had to leave for class before I could get a response back to you, and then I forgot to reply when I got home. |
|
My bad, I misremembered parts of your response when I made mine. You suggested using specifically counting sort ($O(n+k)$) and I was only thinking of sort in general (often $O(n\log(n))$). However, I think then there is the problem of how to sort and store segments. I've run into this problem before: they aren't singular values, they're a combination of a start and an end. Is a segment that starts earlier but ends later "less than" or "greater than" a segment that starts later but ends earlier? Does it "eat" the contained segment? Does a segment that starts earlier and end earlier get merged with a segment whose start intersects that one, but ends later than it? And how do we handle the fact that there are not just an It might be a better idea to use a quad tree. I have yet to implement one successfully, especially when wires are involved, but I think it would be more likely to improve performance than sorting segments every tick since it has inherent locality and positional sorting, and there is empirical evidence that they do boost performance in video games. It would be equally as much work to implement, however. |
|
I've been avoiding your suggestion regarding multithreading, because
|
|
Segment overlap is actually shockingly easy! The gist is that each start pushes onto a stack, and each end marks a collision from the top of the stack to the related start. Then remove that start and continue pushing, marking, and removing. Quad trees are plausible, but I'm not that familiar with their details. I understand the complications with multi-threading. While they have some great benefits with responsiveness, I'd drop them until you have a more optimal single-threaded solution. Lastly, I understand that these are more work, but the false rejection on concave geometries is a blocking bug. If you need some help, guidance, or to be taught some of these algorithms let me know! I understand your class-load. Been there. |
|
I could simply remove the bounding box part for now, if that's what you mean by "false rejection on concave geometries". The bigger speed boost came from the If that's not what you mean, could you please clarify? |
|
The details of a quad tree are that it's much like a binary tree, but in 2D. You split the useable area into quadrants, then separate elements out by "above/below y-midpoint" and "left/right of x-midpoint". You keep splitting up the space until only a manageable number of elements are in each leaf, leaving you with The trouble I have with them is that wires have a tendency of occupying multiple leaves at once... |
I'll try to properly respond to this later in the week. I'm cognitively loaded myself at the moment and can't give you a satisfying and meaningful response. |
|
I wouldn't consider benchmarking at this immediate point in time. Rather,
I'd focus on the core algorithms. But I don't have this fully committed to
memory and could be wrong. I can get back to this late in the week.
…On Tue, Feb 24, 2026, 7:38 AM Amy Wilder ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/jls/edit/SimpleEditor.java
<#4 (comment)>:
> @@ -2821,18 +2822,73 @@ private boolean overlap() {
}*/
+ // Set is O(n log(n)) to traverse over and doesn't benefit from cache locality.
+ // we're doing a lot of iterating over the same collection here,
+ // so it makes sense to use a temporary array for cache locality and O(n) traversal.
+ Element[] selectedArr = selected.toArray(Element[]::new);
+ ArrayList<Element> elementsArr = new ArrayList(circuit.getElements());
+ // elementsArr.removeAll(selected);
+ {
+ int i = 0;
+ while (i < elementsArr.size()) {
+ if (selected.contains(elementsArr.get(i))) {
+ // "swap" remove - slightly cheaper because we don't care about order
I see! I had completely not realized that accessing the end would be what
causes the slowdown, but it makes perfect sense after you explained it!
I'll try benchmarking this and an alternative linked list implementation
and keep whichever is faster. However, I'm not entirely sure how to
benchmark this code without its speed being influenced by the speed of my
own movements. As far as I can tell, there isn't a good way of isolating
the overlap code and running it with a variety specific circuits--I would
have to trigger the overlap method manually. Do you have any suggestions?
—
Reply to this email directly, view it on GitHub
<#4 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXYEBPGXIM2SSSFCEJH6XD4NRA4RAVCNFSM6AAAAACUNDL64GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTQNBXGU4TCNRSG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sets to temporary arrays andArrayLists, since they will be iterated over far more than they will be searched within theoverlapmethod.ArrayListbefore any further checks.i.e.
Related to #3