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 performance of removing elements from sequences #307

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeff303
Copy link
Contributor

@jeff303 jeff303 commented May 25, 2021

Merging InsertBeforeIndex protocol into new IndexedOps protocol for optimized indexed based operations (which now includes inserting before and removing at)

Updating nthpath and keypath to use new helper fns

Adding tests and benchmarks

@jeff303
Copy link
Contributor Author

jeff303 commented May 25, 2021

I think this change should also cover must because it calls do-keypath-transform. Output of new benchmarks from my machine:



Benchmark: set keypath and nthpath at index to NONE versus srange in middle (vector)

Mean(us)	vs best		Code
31.2 		 1.00 		 (setval (nthpath middle-idx) NONE data-vec)
32.1 		 1.03 		 (setval (keypath middle-idx) NONE data-vec)
145 		 4.65 		 (setval (srange middle-idx (inc middle-idx)) [] data-vec)

********************************

Benchmark: set keypath and nthpath at index to NONE versus srange in middle (list)

Mean(us)	vs best		Code
5.56 		 1.00 		 (setval (keypath middle-idx) NONE data-lst)
12.5 		 2.24 		 (setval (srange middle-idx (inc middle-idx)) [] data-lst)
12.7 		 2.29 		 (setval (nthpath middle-idx) NONE data-lst)

********************************

Benchmark: set keypath and nthpath at beginning to NONE versus srange and subvec (vector)

Mean(us)	vs best		Code
0.0179 		 1.00 		 (subvec data-vec 1)
0.150 		 8.39 		 (setval (keypath 0) NONE data-vec)
0.160 		 8.89 		 (setval (nthpath 0) NONE data-vec)
107 		 5.98e+03 		 (setval (srange 0 1) [] data-vec)

********************************

Benchmark: set keypath and nthpath at beginning to NONE versus srange and rest (list)

Mean(us)	vs best		Code
0.00733 		 1.00 		 (rest data-lst)
0.192 		 26.3 		 (setval (keypath 0) NONE data-lst)
12.5 		 1.71e+03 		 (setval (srange 0 1) [] data-lst)
12.8 		 1.75e+03 		 (setval (nthpath 0) NONE data-lst)

********************************

Benchmark: set keypath and nthpath at end to NONE versus srange and subvec (vector)

Mean(us)	vs best		Code
0.0148 		 1.00 		 (subvec data-vec 0 (dec size))
0.257 		 17.4 		 (setval (keypath (dec size)) NONE data-vec)
0.275 		 18.6 		 (setval (nthpath (dec size)) NONE data-vec)
194 		 1.31e+04 		 (setval (srange (dec size) size) [] data-vec)

********************************

@jeff303 jeff303 force-pushed the improve-performance-remove-elements branch from 3a59e44 to b1b952a Compare May 25, 2021 18:53
@jeff303
Copy link
Contributor Author

jeff303 commented May 25, 2021

This should cover most of #219, although it doesn't do anything to address srange performance.

Merging `InsertBeforeIndex` protocol into new `IndexedOps` protocol for optimized indexed based operations (which now includes inserting before and removing at)

Updating `nthpath` and `keypath` to use new helper fns

Adding tests and benchmarks
@jeff303 jeff303 force-pushed the improve-performance-remove-elements branch from b1b952a to 2ace3be Compare May 25, 2021 19:05
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