Skip to content
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

Improve doublylikedlist IndexOf() by reducing the space complexity from O(n) to O(1) #257

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ksw2000
Copy link
Contributor

@ksw2000 ksw2000 commented Jun 21, 2024

Implement IndexOf() without calling Values().

I also checked the performance by running the benchmark I made. Both practical benchmark results and theoretical analysis show that the new implementation significantly improves runtime performance and memory usage

func benchmarkIndexOf(b *testing.B, list *List[int], size int) {
	for i := 0; i < b.N; i++ {
		for n := 0; n < size/4; n++ {
			list.IndexOf(n / 4)
			list.IndexOf(n / 2)
			list.IndexOf(n/4 + n/2)
			list.IndexOf(n - 1)
		}
	}
}

func BenchmarkDoublyLinkedListIndexOf100(b *testing.B) {
	b.StopTimer()
	size := 100
	list := New[int]()
	for n := 0; n < size; n++ {
		list.Add(n)
	}
	b.StartTimer()
	benchmarkIndexOf(b, list, size)
}

func BenchmarkDoublyLinkedListIndexOf1000(b *testing.B) {
	b.StopTimer()
	size := 1000
	list := New[int]()
	for n := 0; n < size; n++ {
		list.Add(n)
	}
	b.StartTimer()
	benchmarkIndexOf(b, list, size)
}

func BenchmarkDoublyLinkedListIndexOf10000(b *testing.B) {
	b.StopTimer()
	size := 10000
	list := New[int]()
	for n := 0; n < size; n++ {
		list.Add(n)
	}
	b.StartTimer()
	benchmarkIndexOf(b, list, size)
}
goos: linux
goarch: amd64
pkg: github.com/emirpasic/gods/v2/lists/doublylinkedlist
cpu: Intel(R) Core(TM) i7-4790 CPU @ 3.60GHz
                               │     old.txt     │               new.txt               │
                               │     sec/op      │   sec/op     vs base                │
DoublyLinkedListIndexOf100-8      86871.0n ± 34%   584.2n ± 1%  -99.33% (p=0.000 n=10)
DoublyLinkedListIndexOf1000-8     9847.72µ ± 19%   84.66µ ± 0%  -99.14% (p=0.000 n=10)
DoublyLinkedListIndexOf10000-8   1220.851m ± 20%   8.908m ± 1%  -99.27% (p=0.000 n=10)
geomean                             10.15m         76.09µ       -99.25%

                               │   old.txt    │                  new.txt                  │
                               │     B/op     │     B/op      vs base                     │
DoublyLinkedListIndexOf100-8     87.50Ki ± 0%    0.00Ki ± 0%  -100.00% (p=0.000 n=10)
DoublyLinkedListIndexOf1000-8    7.813Mi ± 0%   0.000Mi ± 0%  -100.00% (p=0.000 n=10)
DoublyLinkedListIndexOf10000-8   781.3Mi ± 0%     0.0Mi ± 0%  -100.00% (p=0.000 n=10)
geomean                          8.049Mi                      ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

                               │   old.txt   │                 new.txt                  │
                               │  allocs/op  │  allocs/op   vs base                     │
DoublyLinkedListIndexOf100-8      100.0 ± 0%      0.0 ± 0%  -100.00% (p=0.000 n=10)
DoublyLinkedListIndexOf1000-8    1.000k ± 0%   0.000k ± 0%  -100.00% (p=0.000 n=10)
DoublyLinkedListIndexOf10000-8   10.01k ± 0%    0.00k ± 0%  -100.00% (p=0.000 n=10)
geomean                          1.000k                     ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

Implement IndexOf() without calling Values(). Reduce space complexity from O(n) to O(1).
Implement IndexOf() without calling Values(). Reduce space complexity from O(n) to O(1).
@ksw2000
Copy link
Contributor Author

ksw2000 commented Jun 21, 2024

There is the same issue in singlylinkedlist

@ksw2000
Copy link
Contributor Author

ksw2000 commented Jul 8, 2024

Hi @emirpasic
Just a friendly reminder about this PR. It's been two weeks since submission, and I believe the changes would be beneficial to this package. Please let me know if there's anything I need to adjust. Thanks!

@ksw2000
Copy link
Contributor Author

ksw2000 commented Aug 5, 2024

Hi @emirpasic
Just a friendly reminder about this PR. It's been about 6 weeks since submission, and I believe the changes would be beneficial to this package. Please let me know if there's anything I need to adjust. Thanks!

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.

1 participant