Skip to content

Optimize CTQueue for O(1) Remove with Linked List (Fixes #9) #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 2, 2025

Conversation

pankaj-bind
Copy link
Contributor

@pankaj-bind pankaj-bind commented Mar 29, 2025

This PR fixes #9 by replacing OrderedCollection in CTQueue with a singly linked list using CTQueueNode, making remove and poll O(1) instead of O(n).

Changes:

  • Added CTQueueNode class.
  • Refactored CTQueue with head, tail, size.

@pankaj-bind
Copy link
Contributor Author

pankaj-bind commented Mar 29, 2025

@jordanmontt please look for this one

Pharo64-12.xml Outdated
@@ -0,0 +1 @@
<?xml version="1.0" encoding="UTF-8"?><testsuite name="Pharo64-12" tests="13" failures="0" errors="0" time="0.01"> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testAddGarantyFIFOOrder" time="0.002"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testIsEmpty" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testQueueGarantyFIFOOrder" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testPeek" time="0.001"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testAddAll" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testEmptyQueue" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testLargeQueuePerformance" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testAdd" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testQueue" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testRemove" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testEmptyQueueRemove" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testRemoveIfNone" time="0.0"></testcase> <testcase classname="Containers.Queue.Tests.CTQueueTest" name="testPoll" time="0.0"></testcase> <system-out><![CDATA[]]></system-out> <system-err><![CDATA[]]></system-err></testsuite>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this file?

"Verify FIFO behavior and performance with a large queue."
| queue size |
queue := self queueClass new.
size := 1000.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that 1000 is not that large

@jordanmontt
Copy link
Contributor

The code looks good! but can you add more tests? And also, there is a weird file that got added

@coveralls
Copy link

coveralls commented Apr 16, 2025

Pull Request Test Coverage Report for Build 14144595852

Details

  • 60 of 67 (89.55%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 89.773%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Containers-Queue/CTQueue.class.st 48 55 87.27%
Files with Coverage Reduction New Missed Lines %
src/Containers-Queue/CTQueue.class.st 1 88.16%
Totals Coverage Status
Change from base Build 14112397796: -0.02%
Covered Lines: 79
Relevant Lines: 88

💛 - Coveralls

@pankaj-bind
Copy link
Contributor Author

pankaj-bind commented Apr 18, 2025

image

issue: 1 Pharo64-12.xml is a JUnit test report generated by smalltalkCI, detailing 20 passing CTQueueTest tests in Pharo 64-bit v12. It was accidently pushed. I’ll ensure its excluded from future commits.
issue: 2 Increased the queue size in testLargeQueuePerformance to 100,000, which I think is enough to thoroughly test performance.
issue: 3 Added seven new test methods (testAddNilElement, testAddAllLargeCollection, testInterleavedAddRemove, testDoIteration, testIncludes, testSizeAccuracy, testStressAddRemove) to improve test coverage and robustness.

@jordanmontt jordanmontt merged commit 11667b6 into pharo-containers:master May 2, 2025
0 of 4 checks passed
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.

Optimize Performance with a Custom Implementation
3 participants