Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msgpack: Why is float64 used to encode an integer and not int64/uint64? #6328

Closed
nktkas opened this issue Jan 3, 2025 · 3 comments
Closed

Comments

@nktkas
Copy link

nktkas commented Jan 3, 2025

Is your feature request related to a problem? Please describe.

Integers up to 32 bits are encoded in the int/uint type, but for integers of 64 bits, the float64 type is used instead of int64/uint64.

The code sections I'm talking about: https://github.com/denoland/std/blob/main/msgpack/encode.ts#L106 and https://github.com/denoland/std/blob/main/msgpack/encode.ts#L133

Describe the solution you'd like

I want a 64-bit integer to be encoded in int64/unit64 type instead of float64.

Describe alternatives you've considered

You can also look at @msgpack/msgpack, there 64-bit integers are treated as int64/uint64.

As a temporary solution, I convert integer 64 bits to bigint, since bigint is encoded in int64/uint64 type:

import type { ValueType } from "@std/msgpack/encode";

const float64IntegersToUint64 = (obj: ValueType): ValueType => {
    const THIRTY_ONE_BITS = 2147483648;
    const THIRTY_TWO_BITS = 4294967296;
    if (typeof obj === "number" && Number.isInteger(obj) && (obj >= THIRTY_TWO_BITS || obj < -THIRTY_ONE_BITS)) {
        return BigInt(obj); // Now @std/msgpack will handle integer 64 bit as int64/uint64 instead of float64
    } else if (Array.isArray(obj)) {
        return obj.map(float64IntegersToUint64);
    } else if (obj && typeof obj === "object") {
        return Object.fromEntries(
            Object.entries(obj).map(([key, value]) => [key, float64IntegersToUint64(value)]),
        );
    }
    return obj;
};

Note: I'm not very familiar with msgpack, so maybe using float64 instead of int64/int64 is not a bug and I just don't understand something....

@nktkas nktkas changed the title msgpack: Why is float64 used for integer and not int64/uint64? msgpack: Why is float64 used to encode an integer and not int64/uint64? Jan 3, 2025
@BlackAsLight
Copy link
Contributor

I wouldn't say it's a bug. Just an unknown design choice. Probably because it doesn't change how much space the encoding takes up.

On a different note, there is also @std/cbor which offers int64 and uint64 encoding of numbers. It is also faster than @std/msgpack

@nktkas
Copy link
Author

nktkas commented Jan 4, 2025

On a different note, there is also @std/cbor which offers int64 and uint64 encoding of numbers. It is also faster than @std/msgpack

Too bad I need msgpack specifically, cbor is not suitable for my purposes.

I wouldn't say it's a bug. Just an unknown design choice. Probably because it doesn't change how much space the encoding takes up.

You are probably right that this is a purely architectural solution. When decoding uint64/int64, getBigUint64/getBigInt64 is used which always returns bigint. This is a prettier solution, and also preserves compatibility between encode/decode without additional logic.
But I have mixed feelings about it:

  • Adding logic to convert numbers in the range MIN_SAFE_INTEGER and MAX_SAFE_INTEGER to number seems rudimentary against other code.
  • Compatibility with the older @msgpack/msgpack library is broken because of this architectural decision. You can't just replace one library with another, you have to write crutches.

The solution would be to use the msgpack extension, but the issue is still open on this topic.

@nktkas
Copy link
Author

nktkas commented Jan 4, 2025

I think the issue should be closed because it is unlikely that this architectural solution / compatibility with @msgpack/msgpack will be introduced. The only option to solve the problem now is to make a crutch layer for compatibility.

Here is the updated code of my layer if anyone encounters my problem:

import type { ValueType } from "jsr:@std/msgpack/encode";

/**
 * Layer for `@std/msgpack` to create the same byte array for integers as `@msgpack/msgpack`.
 * @param obj - The object to convert integers to BigInt.
 * @returns A new object to use in `@std/msgpack`.
 * @example
 * ```ts
 * import { encode as std_encode } from "jsr:@std/msgpack/encode";
 * import { encode as msgpack_encode } from "npm:@msgpack/msgpack";
 * import { assertEquals, assertNotEquals } from "jsr:@std/assert";
 *
 * const obj = {
 *   a: 4294967296123, // must be uint64, but will be float64 without conversion
 *   b: -4294967296123, // must be int64, but will be float64 without conversion
 *   c: 123456789, // must be uint32
 *   d: -12345, // must be int16
 *   e: -123, // must be int8
 *   f: 123.456, // must be float64
 *   g: 90071992547409900000, // must be float64
 * };
 * const newObj = float64IntegersToUint64(obj);
 * // newObj: {
 * //   a: 4294967296123n,
 * //   b: -4294967296123n,
 * //   c: 123456789,
 * //   d: -12345,
 * //   e: -123,
 * //   f: 123.456,
 * //   g: 90071992547409900000,
 * // }
 *
 * const stdMsgPackBytesWithoutConversion = std_encode(obj);
 * const stdMsgPackBytesWithConversion = std_encode(newObj);
 * const msgPackBytes = msgpack_encode(obj);
 *
 * // You can check msgpack types after encoding here: https://msgpack.dbrgn.ch/
 * assertEquals(stdMsgPackBytesWithConversion, msgPackBytes);
 * assertNotEquals(stdMsgPackBytesWithoutConversion, msgPackBytes);
 * ```
 */
const float64IntegersToUint64 = (obj: ValueType): ValueType => {
    const THIRTY_ONE_BITS = 2147483648;
    const THIRTY_TWO_BITS = 4294967296;
    if (
        typeof obj === "number" && Number.isInteger(obj) && // Number must be integer
        obj <= Number.MAX_SAFE_INTEGER && obj >= Number.MIN_SAFE_INTEGER && // Integer must be in the safe range
        (obj >= THIRTY_TWO_BITS || obj < -THIRTY_ONE_BITS) // Integer must be >= uint32 or < int32
    ) {
        return BigInt(obj); // Now @std/msgpack will create the same byte array as @msgpack/msgpack
    } else if (Array.isArray(obj)) {
        return obj.map(float64IntegersToUint64);
    } else if (obj && typeof obj === "object") {
        return Object.fromEntries(
            Object.entries(obj).map(([key, value]) => [key, float64IntegersToUint64(value)]),
        );
    }
    return obj;
};

(created the issue myself, found the cause myself, found a temporary fix and possible solutions myself xD)

@nktkas nktkas closed this as completed Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants