Skip to content

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> No newline at end of file
Copy link
Member

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
Member

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
Member

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