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

add low level IDR encoding demo #156

Merged
merged 3 commits into from
Jun 18, 2024
Merged

add low level IDR encoding demo #156

merged 3 commits into from
Jun 18, 2024

Conversation

chmike
Copy link
Contributor

@chmike chmike commented Jun 18, 2024

The Information Data Representation (IDR) is a work in progress. This code benchmarks the low level encoding/decoding functions to write custom value encoding/decoding.

The ditp/idr/low package supports full time range with time offset and even time zone abbreviation encoding/decoding. For the benchmark I use the same nsec time encoding as benc.

benchmarks.go Outdated Show resolved Hide resolved
@matheusd
Copy link
Contributor

The ditp/idr/low package supports full time range with time offset and even time zone abbreviation encoding/decoding.

If the low level package supports it natively (i.e. low.PutTime...) I'd suggest using that instead of nsec to demonstrate it

@deneonet
Copy link
Collaborator

For demonstration, you should also rerun the benchmarks as instructed in the README.

@chmike
Copy link
Contributor Author

chmike commented Jun 18, 2024

I have applied your change requests and your suggestion to use the full time with tz offset.

But I don't see the point with the time encoding because we end up comparing apples with oranges. It is obviously slower and bigger with the full time encoding.

@chmike
Copy link
Contributor Author

chmike commented Jun 18, 2024

I did run the benchmark. Do you want me to commit it as well ?

@deneonet
Copy link
Collaborator

Yes.

@deneonet deneonet merged commit 0d9a3f2 into alecthomas:master Jun 18, 2024
@chmike
Copy link
Contributor Author

chmike commented Jun 18, 2024

Thank you.

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

Successfully merging this pull request may close these issues.

3 participants