test: add reverseiterator tests

[wolkykim]

- update memiterator to use a cursor instead of reslicing in the iteration loop that also eliminates repeated if ascending check

• add tests for reverse iterator

[piux2]

Thank you contribution!

Which line of code will trigger underline array copying?

the original Next() automatically validate the boundary during reslicing. new changes could leave the cur dangling out of the boundary.

[wolkykim]

Hi,

Which line of code will trigger underline array copying?

I realize the term copying is probably not a great way to describe it.
It's more of the reslicing operation in the iteration that doesn't necessarily involve an actual copy of the data array.

This is the line that made me work: https://github.com/gnolang/gno/pull/1058/files#diff-357b5cf1fe3b41c11ec07a21d11364079ac798c1b6d7817df02a2065f3e5a9b4L59

Also, this repeated 'if ascending` checks:
https://github.com/gnolang/gno/pull/1058/files#diff-357b5cf1fe3b41c11ec07a21d11364079ac798c1b6d7817df02a2065f3e5a9b4L67
https://github.com/gnolang/gno/pull/1058/files#diff-357b5cf1fe3b41c11ec07a21d11364079ac798c1b6d7817df02a2065f3e5a9b4L75

the original Next() automatically validate the boundary during reslicing. new changes could leave the cur dangling out of the boundary.

I don't think I follow exactly what you meant. But here's my view:

About the cur pointing out of boundary: New logic's cur value will have either len() or -1 at the end of the iteration if this was what you meant by dangling out of the boundary, that's intended and such condition is used to check in Valid() method. Since the cur value is not exposed to users, I don't get to see what the concerns would be around that.

About validation. Both logic relies on mi.assertValid() for validation. Original logic must do it otherwise real panic will happen on reslicing. It's an option for new logic but I left it there to keep it behaving the same.

Personally, I'd prefer not to have Valid() method but only Next() if it's possible to refactor the behavior but that's out of this PR range.

Thanks for the feedback. Let me know what you think @piux2

[michelleellen]

also @wolkykim if your interested in joining the team for our regular calls please let me know.

[piux2]

@wolkykim thank you for the explanation, your testing case is good! please see my comments below and let me know what you think.

The iterator functions as a memcache iterator, with the underlying data structure being a slice, []items, that contains pointers to the key-value pairs. When the Next() method is called, the length of the []items slice decreases (by executing mi.items = mi.items[1:]).

Although the underlying array remains unaltered, any memory pointed by *std.KVPair that is stored outside the current length of the slice can become eligible for garbage collection by Go's memory management, assuming no secondary references to it exist elsewhere.

This design is optimized for cache applications, ensuring that the iterator doesn't retain extra references to a *std.KVPair once it's been iterated over.

Also, I do not see the expected performance improvement.
The benchmark testing case is in tm2/pkg/store/cache/

I noticed -2.2% to -5.2% performance hit with the new implementation. I run 5 times for each comparison and received consistent outcome.

here is the result of 1 of 5 comparisons

existing implementation:

goos: darwin
goarch: arm64
pkg: github.com/gnolang/gno/tm2/pkg/store/cache
BenchmarkCacheStoreIterator500-8      	   76245	     15650 ns/op
BenchmarkCacheStoreIterator1000-8     	   37345	     32047 ns/op
BenchmarkCacheStoreIterator10000-8    	    3265	    365832 ns/op
BenchmarkCacheStoreIterator50000-8    	     530	   1941289 ns/op
BenchmarkCacheStoreIterator100000-8   	     238	   4481722 ns/op
BenchmarkCacheStoreGetNoKeyFound-8    	 5090340	       285.7 ns/op
BenchmarkCacheStoreGetKeyFound-8      	11871750	       130.4 ns/op

new implementation:

goos: darwin
goarch: arm64
pkg: github.com/gnolang/gno/tm2/pkg/store/cache
BenchmarkCacheStoreIterator500-8      	   72349	     16501 ns/op
BenchmarkCacheStoreIterator1000-8     	   35344	     33844 ns/op
BenchmarkCacheStoreIterator10000-8    	    3144	    378636 ns/op
BenchmarkCacheStoreIterator50000-8    	     511	   2016924 ns/op
BenchmarkCacheStoreIterator100000-8   	     235	   4583330 ns/op
BenchmarkCacheStoreGetNoKeyFound-8    	 5033695	       286.5 ns/op
BenchmarkCacheStoreGetKeyFound-8      	11924443	       130.7 ns/op

[wolkykim]

@piux2 Hi, yeah........ I see that the cursor approach didn't help the performance at all and actually made it slower. I ran the benchmark on my end and saw the same result. Also, see that your laptop has a faster CPU than mine. I'm surprised to learn that reslicing cost is that cheap. Thank you and I'm sorry for having your time spent on this. I have reverted the iterator change. This PR is now about adding the ReveseIterator test cases. Still lives 🤘

[piux2]

@piux2 Hi, yeah........ I see that the cursor approach didn't help the performance at all and actually made it slower. I ran the benchmark on my end and saw the same result. Also, see that your laptop has a faster CPU than mine. I'm surprised to learn that reslicing cost is that cheap. Thank you and I'm sorry for having your time spent on this. I have reverted the iterator change. This PR is now about adding the ReveseIterator test cases. Still lives 🤘

@wolkykim awesome!