golang/go

internal/zstd: refactoring literals.go implementation to add decodeSymbol method

Open

#64,031 opened on Nov 9, 2023

View on GitHub
 (4 comments) (0 reactions) (0 assignees)Go (19,008 forks)batch import
NeedsInvestigationcompiler/runtimehelp wanted

Repository metrics

Stars
 (133,883 stars)
PR merge metrics
 (No merged PRs in 30d)

Description

In the zstd implementation, readLiteralsOneStream and readLiteralsFourStreams duplicate a lot of code, and for the sake of code maintainability, I suggest implementing the duplicated code into a decodeSymbol method.

The duplicate code is available at https://github.com/golang/go/blob/master/src/internal/zstd/literals.go#L283-L326

fetchHuff := func(rbr *reverseBitReader) (uint16, error) {
	if !rbr.fetch(uint8(huffBits)) {
		return 0, rbr.makeError("literals Huffman stream out of bits")
	}
	idx := (rbr.bits >> (rbr.cnt - huffBits)) & huffMask
	return huffTable[idx], nil
}
t1, err := fetchHuff(&rbr1)
if err != nil {
	return nil, err
}
outbuf[out1] = byte(t1 >> 8)
out1++
rbr1.cnt -= uint32(t1 & 0xff)

This part of the code is used between readLiteralsOneStream and readLiteralsFourStreams. According to RFC 8878 3.1.1.3.1.6

Each of these 4 bitstreams is then decoded independently as a Huffman-coded stream, as described in Section 4.2.2.

The logic of their implementation is exactly the same, a decodeSymbol method can be implemented

func decodeSymbol(huffBits, huffMask uint32, huffTable []uint16, rbr *reverseBitReader, outbuf []byte, off int) (int, error) {
	if !rbr.fetch(uint8(huffBits)) {
		return 0, rbr.makeError("literals Huffman stream out of bits")
	}
	idx := (rbr.bits >> (rbr.cnt - huffBits)) & huffMask
	t := huffTable[idx]
	outbuf[off] = byte(t >> 8)
	off++
	rbr.cnt -= uint32(t & 0xff)
	return off, nil
}

The decodeSymbol can be implemented in readLiteralsFourStreams and readLiteralsOneStream using the simplified code

Contributor guide