Repository: valkey-io/valkey
File: src/t_zset.c (C)

Code before:
        x->level[level].span -= decr;
}

static inline unsigned long zslGetNodeHeight(const zskiplistNode *x) {
    /* Since the span at level 0 is always 1 (or 0 for the last node), this
     * field is instead used for storing the height of the node. */
    return x->level[0].span;
}

static inline void zslSetNodeHeight(zskiplistNode *x, int height) {
    /* Since the span at level 0 is always 1 (or 0 for the last node), this
     * field is instead used for storing the height of the node. */
    x->level[0].span = height;
}

/* Create a skiplist node with the specified number of levels.
 * By embedding elements and levels into the skiplist nodes,
 * we achieve good cache-friendliness and a compact memory structure.
 *
 * The memory layout is as follows:
 *
 *   +-------+------------------+---------+-----+---------+-----------------+-------------+
 *   | score | backward-pointer | level-0 | ... | level-N | sds-header-size | element-sds |
 *   +-------+------------------+---------+-----+---------+-----------------+-------------+
 *   
 *   sds-header-size and element-sds are only valid for non-header nodes.
 */
static zskiplistNode *zslCreateNode(int height, double score, const_sds ele) {
    size_t ele_sds_len = ele ? sdslen(ele) : 0;
    char ele_sds_type = ele ? sdsReqType(ele_sds_len) : 0;
    size_t ele_sds_size = ele ? sdsReqSize(ele_sds_len, ele_sds_type) : 0;
    /* Allocate enough space for the node, levels, and the element sds.
     * For non-header nodes, we include one extra byte representing the sds
     * header size, which is the offset into the embedded sds data where the
     * string content starts. */
    size_t header_size = ele ? 1 : 0;
    zskiplistNode *zn = zmalloc(sizeof(*zn) + height * sizeof(struct zskiplistLevel) + header_size + ele_sds_size);
    zn->score = score;
    zslSetNodeHeight(zn, height);
    if (ele) {
        char *data = ((char *)(zn + 1)) + height * sizeof(struct zskiplistLevel);
        *data++ = sdsHdrSize(ele_sds_type);
        sdswrite(data, ele_sds_size, ele_sds_type, ele, ele_sds_len);
    }
    return zn;
}

/* Helper function to return the element string from a skip list node. */
sds zslGetNodeElement(const zskiplistNode *x) {
    unsigned char *data = (void *)(x + 1);
    data += zslGetNodeHeight(x) * sizeof(struct zskiplistLevel);

Review comment (suggestion):
There is now some difference between header and non-header nodes. Maybe we should create a separate function for the header nodes? Then we can require that `ele` is always set here and remove the conditional code that checks if ele is NULL or not.

zslCreateHeaderNode is very simple:

```C
static zskiplistNode *zslCreateHeaderNode(void) {
    /* Allocate enough space for the node and levels. */
    zskiplistNode *zn = zmalloc(sizeof(*zn) + ZSKIPLIST_MAXLEVEL * sizeof(struct zskiplistLevel));
    zslSetNodeHeight(zn, ZSKIPLIST_MAXLEVEL);
    return zn;
}
```

Code after:
        x->level[level].span -= decr;
}

static inline unsigned long zslGetNodeHeight(const zskiplistNode *x) {
    /* Since the span at level 0 is always 1 (or 0 for the last node), this
     * field is instead used for storing the height of the node. */
    return x->level[0].span;
}

static inline void zslSetNodeHeight(zskiplistNode *x, int height) {
    /* Since the span at level 0 is always 1 (or 0 for the last node), this
     * field is instead used for storing the height of the node. */
    x->level[0].span = height;
}

/* Create a skiplist node with the specified number of levels.
 * By embedding elements and levels into the skiplist nodes,
 * we achieve good cache-friendliness and a compact memory structure.
 *
 * The memory layout is as follows:
 *
 *   +-------+------------------+---------+-----+---------+-----------------+-------------+
 *   | score | backward-pointer | level-0 | ... | level-N | sds-header-size | element-sds |
 *   +-------+------------------+---------+-----+---------+-----------------+-------------+
 *
 *   sds-header-size and element-sds are only valid for non-header nodes.
 */
static zskiplistNode *zslCreateNode(int height, double score, const_sds ele) {
    size_t ele_sds_len = sdslen(ele);
    char ele_sds_type = sdsReqType(ele_sds_len);
    size_t ele_sds_size = sdsReqSize(ele_sds_len, ele_sds_type);
    /* Allocate enough space for the node, levels, and the element sds.
     * We include one extra byte representing the sds header size,
     * which is the offset into the embedded sds data where the
     * string content starts. */
    zskiplistNode *zn = zmalloc(sizeof(*zn) + height * sizeof(struct zskiplistLevel) + 1 + ele_sds_size);
    zn->score = score;
    zslSetNodeHeight(zn, height);
    char *data = ((char *)(zn + 1)) + height * sizeof(struct zskiplistLevel);
    *data++ = sdsHdrSize(ele_sds_type);
    sdswrite(data, ele_sds_size, ele_sds_type, ele, ele_sds_len);
    return zn;
}

static zskiplistNode *zslCreateHeaderNode(void) {
    /* Allocate enough space for the node and levels. */
    zskiplistNode *zn = zmalloc(sizeof(*zn) + ZSKIPLIST_MAXLEVEL * sizeof(struct zskiplistLevel));
    zslSetNodeHeight(zn, ZSKIPLIST_MAXLEVEL);
    return zn;
}