4940: Refactor CHuffman r=def- a=Robyt3

Some refactorings I also PR'ed on upstream (https://github.com/teeworlds/teeworlds/pull/3138), and some changes from 43a247e24d that were not on ddnet yet. The `CHuffman` classes from ddnet and upstream should be identical now apart from code format.

On upstream, 43a247e24d  also changes how CHuffman is used. Previously a singleton `CHuffman` instance was encapsulated in `CNetBase`. On upstream this commit duplicates the huffman instance, which I'd like to avoid, so I did not port that change to ddnet. The cleaner solution would be to create an `IHuffman` engine interface so it can be registered with the kernel.

Replacing bubble sort with `std::stable_sort` should reduce client startup time by around 0,5ms! 🚀 

## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [X] Considered possible null pointers and out of bounds array indexing
- [X] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>
This commit is contained in:
bors[bot] 2022-04-02 14:57:18 +00:00 committed by GitHub
commit fc1b54e4e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 70 deletions

View file

@ -1,14 +1,35 @@
/* (c) Magnus Auvinen. See licence.txt in the root of the distribution for more information. */ /* (c) Magnus Auvinen. See licence.txt in the root of the distribution for more information. */
/* If you are missing that file, acquire a complete release at teeworlds.com. */ /* If you are missing that file, acquire a complete release at teeworlds.com. */
#include "huffman.h" #include "huffman.h"
#include <algorithm>
#include <base/system.h> #include <base/system.h>
const unsigned CHuffman::ms_aFreqTable[HUFFMAN_MAX_SYMBOLS] = {
1 << 30, 4545, 2657, 431, 1950, 919, 444, 482, 2244, 617, 838, 542, 715, 1814, 304, 240, 754, 212, 647, 186,
283, 131, 146, 166, 543, 164, 167, 136, 179, 859, 363, 113, 157, 154, 204, 108, 137, 180, 202, 176,
872, 404, 168, 134, 151, 111, 113, 109, 120, 126, 129, 100, 41, 20, 16, 22, 18, 18, 17, 19,
16, 37, 13, 21, 362, 166, 99, 78, 95, 88, 81, 70, 83, 284, 91, 187, 77, 68, 52, 68,
59, 66, 61, 638, 71, 157, 50, 46, 69, 43, 11, 24, 13, 19, 10, 12, 12, 20, 14, 9,
20, 20, 10, 10, 15, 15, 12, 12, 7, 19, 15, 14, 13, 18, 35, 19, 17, 14, 8, 5,
15, 17, 9, 15, 14, 18, 8, 10, 2173, 134, 157, 68, 188, 60, 170, 60, 194, 62, 175, 71,
148, 67, 167, 78, 211, 67, 156, 69, 1674, 90, 174, 53, 147, 89, 181, 51, 174, 63, 163, 80,
167, 94, 128, 122, 223, 153, 218, 77, 200, 110, 190, 73, 174, 69, 145, 66, 277, 143, 141, 60,
136, 53, 180, 57, 142, 57, 158, 61, 166, 112, 152, 92, 26, 22, 21, 28, 20, 26, 30, 21,
32, 27, 20, 17, 23, 21, 30, 22, 22, 21, 27, 25, 17, 27, 23, 18, 39, 26, 15, 21,
12, 18, 18, 27, 20, 18, 15, 19, 11, 17, 33, 12, 18, 15, 19, 18, 16, 26, 17, 18,
9, 10, 25, 22, 22, 17, 20, 16, 6, 16, 15, 20, 14, 18, 24, 335, 1517};
struct CHuffmanConstructNode struct CHuffmanConstructNode
{ {
unsigned short m_NodeId; unsigned short m_NodeId;
int m_Frequency; int m_Frequency;
}; };
bool CompareNodesByFrequencyDesc(const CHuffmanConstructNode *pNode1, const CHuffmanConstructNode *pNode2)
{
return pNode2->m_Frequency < pNode1->m_Frequency;
}
void CHuffman::Setbits_r(CNode *pNode, int Bits, unsigned Depth) void CHuffman::Setbits_r(CNode *pNode, int Bits, unsigned Depth)
{ {
if(pNode->m_aLeafs[1] != 0xffff) if(pNode->m_aLeafs[1] != 0xffff)
@ -23,29 +44,6 @@ void CHuffman::Setbits_r(CNode *pNode, int Bits, unsigned Depth)
} }
} }
// TODO: this should be something faster, but it's enough for now
static void BubbleSort(CHuffmanConstructNode **ppList, int Size)
{
int Changed = 1;
CHuffmanConstructNode *pTemp;
while(Changed)
{
Changed = 0;
for(int i = 0; i < Size - 1; i++)
{
if(ppList[i]->m_Frequency < ppList[i + 1]->m_Frequency)
{
pTemp = ppList[i];
ppList[i] = ppList[i + 1];
ppList[i + 1] = pTemp;
Changed = 1;
}
}
Size--;
}
}
void CHuffman::ConstructTree(const unsigned *pFrequencies) void CHuffman::ConstructTree(const unsigned *pFrequencies)
{ {
CHuffmanConstructNode aNodesLeftStorage[HUFFMAN_MAX_SYMBOLS]; CHuffmanConstructNode aNodesLeftStorage[HUFFMAN_MAX_SYMBOLS];
@ -73,8 +71,7 @@ void CHuffman::ConstructTree(const unsigned *pFrequencies)
// construct the table // construct the table
while(NumNodesLeft > 1) while(NumNodesLeft > 1)
{ {
// we can't rely on stdlib's qsort for this, it can generate different results on different implementations std::stable_sort(apNodesLeft, apNodesLeft + NumNodesLeft, CompareNodesByFrequencyDesc);
BubbleSort(apNodesLeft, NumNodesLeft);
m_aNodes[m_NumNodes].m_NumBits = 0; m_aNodes[m_NumNodes].m_NumBits = 0;
m_aNodes[m_NumNodes].m_aLeafs[0] = apNodesLeft[NumNodesLeft - 1]->m_NodeId; m_aNodes[m_NumNodes].m_aLeafs[0] = apNodesLeft[NumNodesLeft - 1]->m_NodeId;
@ -95,16 +92,17 @@ void CHuffman::ConstructTree(const unsigned *pFrequencies)
void CHuffman::Init(const unsigned *pFrequencies) void CHuffman::Init(const unsigned *pFrequencies)
{ {
int i;
// make sure to cleanout every thing // make sure to cleanout every thing
mem_zero(this, sizeof(*this)); mem_zero(m_aNodes, sizeof(m_aNodes));
mem_zero(m_apDecodeLut, sizeof(m_apDecodeLut));
m_pStartNode = 0x0;
m_NumNodes = 0;
// construct the tree // construct the tree
ConstructTree(pFrequencies); ConstructTree(pFrequencies);
// build decode LUT // build decode LUT
for(i = 0; i < HUFFMAN_LUTSIZE; i++) for(int i = 0; i < HUFFMAN_LUTSIZE; i++)
{ {
unsigned Bits = i; unsigned Bits = i;
int k; int k;
@ -130,7 +128,7 @@ void CHuffman::Init(const unsigned *pFrequencies)
} }
//*************************************************************** //***************************************************************
int CHuffman::Compress(const void *pInput, int InputSize, void *pOutput, int OutputSize) int CHuffman::Compress(const void *pInput, int InputSize, void *pOutput, int OutputSize) const
{ {
// this macro loads a symbol for a byte into bits and bitcount // this macro loads a symbol for a byte into bits and bitcount
#define HUFFMAN_MACRO_LOADSYMBOL(Sym) \ #define HUFFMAN_MACRO_LOADSYMBOL(Sym) \
@ -197,7 +195,7 @@ int CHuffman::Compress(const void *pInput, int InputSize, void *pOutput, int Out
} }
//*************************************************************** //***************************************************************
int CHuffman::Decompress(const void *pInput, int InputSize, void *pOutput, int OutputSize) int CHuffman::Decompress(const void *pInput, int InputSize, void *pOutput, int OutputSize) const
{ {
// setup buffer pointers // setup buffer pointers
unsigned char *pDst = (unsigned char *)pOutput; unsigned char *pDst = (unsigned char *)pOutput;
@ -208,8 +206,8 @@ int CHuffman::Decompress(const void *pInput, int InputSize, void *pOutput, int O
unsigned Bits = 0; unsigned Bits = 0;
unsigned Bitcount = 0; unsigned Bitcount = 0;
CNode *pEof = &m_aNodes[HUFFMAN_EOF_SYMBOL]; const CNode *pEof = &m_aNodes[HUFFMAN_EOF_SYMBOL];
CNode *pNode = 0; const CNode *pNode = 0;
while(true) while(true)
{ {

View file

@ -30,6 +30,8 @@ class CHuffman
unsigned char m_Symbol; unsigned char m_Symbol;
}; };
static const unsigned ms_aFreqTable[HUFFMAN_MAX_SYMBOLS];
CNode m_aNodes[HUFFMAN_MAX_NODES]; CNode m_aNodes[HUFFMAN_MAX_NODES];
CNode *m_apDecodeLut[HUFFMAN_LUTSIZE]; CNode *m_apDecodeLut[HUFFMAN_LUTSIZE];
CNode *m_pStartNode; CNode *m_pStartNode;
@ -40,49 +42,46 @@ class CHuffman
public: public:
/* /*
Function: huffman_init Function: Init
Inits the compressor/decompressor. Inits the compressor/decompressor.
Parameters: Parameters:
huff - Pointer to the state to init pFrequencies - A pointer to an array of 256 entries of the frequencies of the bytes
frequencies - A pointer to an array of 256 entries of the frequencies of the bytes
Remarks: Remarks:
- Does no allocation what so ever. - Does no allocation whatsoever.
- You don't have to call any cleanup functions when you are done with it - You don't have to call any cleanup functions when you are done with it.
*/ */
void Init(const unsigned *pFrequencies); void Init(const unsigned *pFrequencies = ms_aFreqTable);
/* /*
Function: huffman_compress Function: Compress
Compresses a buffer and outputs a compressed buffer. Compresses a buffer and outputs a compressed buffer.
Parameters: Parameters:
huff - Pointer to the huffman state pInput - Buffer to compress
input - Buffer to compress InputSize - Size of the buffer to compress
input_size - Size of the buffer to compress pOutput - Buffer to put the compressed data into
output - Buffer to put the compressed data into OutputSize - Size of the output buffer
output_size - Size of the output buffer
Returns: Returns:
Returns the size of the compressed data. Negative value on failure. Returns the size of the compressed data. Negative value on failure.
*/ */
int Compress(const void *pInput, int InputSize, void *pOutput, int OutputSize); int Compress(const void *pInput, int InputSize, void *pOutput, int OutputSize) const;
/* /*
Function: huffman_decompress Function: Decompress
Decompresses a buffer Decompresses a buffer
Parameters: Parameters:
huff - Pointer to the huffman state pInput - Buffer to decompress
input - Buffer to decompress InputSize - Size of the buffer to decompress
input_size - Size of the buffer to decompress pOutput - Buffer to put the uncompressed data into
output - Buffer to put the uncompressed data into OutputSize - Size of the output buffer
output_size - Size of the output buffer
Returns: Returns:
Returns the size of the uncompressed data. Negative value on failure. Returns the size of the uncompressed data. Negative value on failure.
*/ */
int Decompress(const void *pInput, int InputSize, void *pOutput, int OutputSize); int Decompress(const void *pInput, int InputSize, void *pOutput, int OutputSize) const;
}; };
#endif // __HUFFMAN_HEADER__ #endif // ENGINE_SHARED_HUFFMAN_H

View file

@ -412,22 +412,7 @@ int CNetBase::Decompress(const void *pData, int DataSize, void *pOutput, int Out
return ms_Huffman.Decompress(pData, DataSize, pOutput, OutputSize); return ms_Huffman.Decompress(pData, DataSize, pOutput, OutputSize);
} }
static const unsigned s_aFreqTable[256 + 1] = {
1 << 30, 4545, 2657, 431, 1950, 919, 444, 482, 2244, 617, 838, 542, 715, 1814, 304, 240, 754, 212, 647, 186,
283, 131, 146, 166, 543, 164, 167, 136, 179, 859, 363, 113, 157, 154, 204, 108, 137, 180, 202, 176,
872, 404, 168, 134, 151, 111, 113, 109, 120, 126, 129, 100, 41, 20, 16, 22, 18, 18, 17, 19,
16, 37, 13, 21, 362, 166, 99, 78, 95, 88, 81, 70, 83, 284, 91, 187, 77, 68, 52, 68,
59, 66, 61, 638, 71, 157, 50, 46, 69, 43, 11, 24, 13, 19, 10, 12, 12, 20, 14, 9,
20, 20, 10, 10, 15, 15, 12, 12, 7, 19, 15, 14, 13, 18, 35, 19, 17, 14, 8, 5,
15, 17, 9, 15, 14, 18, 8, 10, 2173, 134, 157, 68, 188, 60, 170, 60, 194, 62, 175, 71,
148, 67, 167, 78, 211, 67, 156, 69, 1674, 90, 174, 53, 147, 89, 181, 51, 174, 63, 163, 80,
167, 94, 128, 122, 223, 153, 218, 77, 200, 110, 190, 73, 174, 69, 145, 66, 277, 143, 141, 60,
136, 53, 180, 57, 142, 57, 158, 61, 166, 112, 152, 92, 26, 22, 21, 28, 20, 26, 30, 21,
32, 27, 20, 17, 23, 21, 30, 22, 22, 21, 27, 25, 17, 27, 23, 18, 39, 26, 15, 21,
12, 18, 18, 27, 20, 18, 15, 19, 11, 17, 33, 12, 18, 15, 19, 18, 16, 26, 17, 18,
9, 10, 25, 22, 22, 17, 20, 16, 6, 16, 15, 20, 14, 18, 24, 335, 1517};
void CNetBase::Init() void CNetBase::Init()
{ {
ms_Huffman.Init(s_aFreqTable); ms_Huffman.Init();
} }