Chunk iterator performance improvement

[joe-elliott]

What this PR does / why we need it:
Improves the headblock iterator performance by storing an index instead of a current item.

Before:

go test -bench=BenchmarkHead. -benchmem
goos: linux
goarch: amd64
pkg: github.com/grafana/loki/pkg/chunkenc
BenchmarkHeadBlockIterator/Size_100000-8         	     501	   2384544 ns/op	 2400311 B/op	       2 allocs/op
BenchmarkHeadBlockIterator/Size_50000-8          	     916	   1155438 ns/op	 1204272 B/op	       2 allocs/op
BenchmarkHeadBlockIterator/Size_15000-8          	    3272	    331680 ns/op	  360496 B/op	       2 allocs/op
BenchmarkHeadBlockIterator/Size_10000-8          	    5660	    232224 ns/op	  245808 B/op	       2 allocs/op
PASS
ok  	github.com/grafana/loki/pkg/chunkenc	8.534s

After

go test -bench=BenchmarkHead. -benchmem
goos: linux
goarch: amd64
pkg: github.com/grafana/loki/pkg/chunkenc
BenchmarkHeadBlockIterator/Size_100000-8         	     486	   2430331 ns/op	 2400288 B/op	       2 allocs/op
BenchmarkHeadBlockIterator/Size_50000-8          	    1070	   1126875 ns/op	 1204256 B/op	       2 allocs/op
BenchmarkHeadBlockIterator/Size_15000-8          	    4156	    279365 ns/op	  360480 B/op	       2 allocs/op
BenchmarkHeadBlockIterator/Size_10000-8          	    6690	    177187 ns/op	  245792 B/op	       2 allocs/op
PASS
ok  	github.com/grafana/loki/pkg/chunkenc	8.675s

Special notes for your reviewer:
Please review and discuss the guard code at the top of Entry() would we prefer a panic to returning dummy data here?

	if li.cur < 0 || li.cur >= len(li.entries) {
		return logproto.Entry{}
	}

Checklist

• Documentation added
• Tests updated

[cyriltovena]

Why did you close that one ? It was looking good.

[joe-elliott]

Updated the benchmarks to be more comprehensive:

100k -     - 243 / 238 = 1.02
50k  - p99 - 112 / 114 = .98
15k  - avg -  27 /  31 = .81
10k  - p50 -  17 /  23 = .73

@cyriltovena I just should have waited til Monday. I was unsure of the validity of the benchmarks and wanted to feel more comfortable before someone merged.

[owen-d]

Unsure regarding panic vs returning the zero-value struct, but I believe your approach is good. I know that in cortex, for instance, we assume that entries will not be checked when Next() is false. Doing so would cause a panic, so perhaps your code should mimic that behavior. However, I like your approach of avoiding panicking where possible and leaving it up to the consumer to use it correctly.

:lgtm:

[joe-elliott]

@owen-d
Right. I think the "correct" approach is to not have the guard code, but given how important the stability of the ingesters are I decided to add it as a discussion point.

[cyriltovena]

LGTM