Skip to content

fix: avoid creating a dict and hashing boolean values - #20

Closed
ThomasByr wants to merge 2 commits into
rsalmei:mainfrom
ThomasByr:main
Closed

fix: avoid creating a dict and hashing boolean values#20
ThomasByr wants to merge 2 commits into
rsalmei:mainfrom
ThomasByr:main

Conversation

@ThomasByr

Copy link
Copy Markdown
Contributor

Hi!

While reading the source code, I found that in about_time/features.py, the way conv_space is written adds unnecessary latency. While being nice, indexing and creating a dictionary each time can waste some time.

In fact, this can be measured.
def hash_stuff(value: bool):
    return {True: " ", False: ""}[value]


def return_stuff(value: bool):
    return " " if value else ""


def old_return_stuff(value: bool):
    if value:
        return " "
    else:
        return ""


def index_stuff(value: bool):
    return ["", " "][value]


cache = {True: " ", False: ""}


def hash_cached_stuff(value: bool):
    return cache[value]


if __name__ == "__main__":
    from random import randint

    from about_time import about_time

    N = 100_000_000
    Y = [bool(randint(0, 1)) for _ in range(N)]

    with about_time() as t0:
        for x in Y:
            hash_stuff(x)

    with about_time() as t1:
        for x in Y:
            return_stuff(x)

    with about_time() as t2:
        for x in Y:
            index_stuff(x)

    with about_time() as t3:
        for x in Y:
            hash_cached_stuff(x)

    with about_time() as t4:
        for x in Y:
            old_return_stuff(x)

    print(f"Hash time:         {t0.duration_human}")
    print(f"Return time:       {t1.duration_human}")
    print(f"Index time:        {t2.duration_human}")
    print(f"Cached Hash time:  {t3.duration_human}")
    print(f"Old Return time:   {t4.duration_human}")
Hash time:         12.95s
Return time:       6.2s
Index time:        9.23s
Cached Hash time:  7.03s
Old Return time:   6.13s

While ternary operations perform slightly worse than just checking (which is obscure to me), I think we can live with it to still have a one-liner.

@rsalmei

rsalmei commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Thanks, @ThomasByr. But I believe it’s clearer how it currently stands (perhaps not the if one, but definitely the matrix).

@rsalmei rsalmei closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants