In compressSingleKeyValue(), we compute the number of bytes to store
timestamp:

      timestampFitsInBytes = ByteBufferUtils.longFitsIn(timestamp);

Then the number of bytes to store the diff between the timestamp of
previous cell and current timestamp is computed:

      diffTimestampFitsInBytes = ByteBufferUtils.longFitsIn(diffTimestamp);

After that FLAG_TIMESTAMP_IS_DIFF would be set if it is more efficient to
store the diff:

      if (diffTimestampFitsInBytes < timestampFitsInBytes) {

        flag |= (diffTimestampFitsInBytes - 1) << SHIFT_TIMESTAMP_LENGTH;

        flag |= FLAG_TIMESTAMP_IS_DIFF;

This should achieve good efficiency.

Cheers

On Sun, Sep 18, 2016 at 9:37 AM, Lars George <lars.geo...@gmail.com> wrote:

> Hi,
>
> Reading the key encoder code and noticed this in the DiffKeyDeltaEncoder:
>
> if ((flag & FLAG_TIMESTAMP_IS_DIFF) == 0) {
>   ByteBufferUtils.putLong(out, timestamp, timestampFitsInBytes);
> } else {
>   ByteBufferUtils.putLong(out, diffTimestamp, diffTimestampFitsInBytes);
> }
>
> So if the timestamp is the same as the one from the previous cell, we
> store it again in its full fidelity? This makes no sense as we omit
> otherwise all identical fields. I would assume this is a mistake?
>
> In the decoding we do this:
>
> // handle timestamp
> int timestampFitsInBytes =
>     ((flag & MASK_TIMESTAMP_LENGTH) >>> SHIFT_TIMESTAMP_LENGTH) + 1;
> long timestamp = ByteBufferUtils.readLong(source, timestampFitsInBytes);
> if ((flag & FLAG_TIMESTAMP_SIGN) != 0) {
>   timestamp = -timestamp;
> }
> if ((flag & FLAG_TIMESTAMP_IS_DIFF) != 0) {
>   timestamp = state.timestamp - timestamp;
> }
> buffer.putLong(timestamp);
>
> This could be changed to simply use the state.timestamp if the value
> is the same, no? Right now we would add six bytes for the cell
> timestamp when they are identical.
>
> If they are not, we store delta time, so I guess in practice this does
> not happen too often, that we have the same time, unless they are
> client supplied, say for a row to emulate transactions.
>
> Should I open a ticket?
>
> Cheers,
> Lars
>

Reply via email to