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

Getting, then later writing, a symbol's ID with v1.1 BinaryWriter (redux of #888) #893

Open
barries opened this issue Jan 7, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@barries
Copy link

barries commented Jan 7, 2025

This feature request is to add a method like SymbolTable::get_or_insert()[1] to allow callers to intern symbols that will be reused. For example, in the context of emitting a telemetry stream, there are enums used in every "log entry", like log level, log entry kind, that are known at compile time, and there are frequently used symbols that are computed at runtime, then re-used, like the name of the currently executing function.

Currently the only way I can find to do this is to write_symbol(my_str), then look my_str up in symbol_table() to get the SymbolID; this is less efficient and more awkward than necessary.

Example:

let symbol_id = ion_writer.symbol_table().get_or_insert(my_str);
ion_writer.write(symbol_id)
... time passes ...
ion_writer.write(symbol_id)

[1] This is the functionality I meant to request in #888, but which wasn't clear enough about--the first example showed it, but the second one wasn't as clear as it should have been (sorry!).

@barries barries added the enhancement New feature or request label Jan 7, 2025
@zslayton
Copy link
Contributor

zslayton commented Jan 8, 2025

Sorry I missed your question on the merged PR!

Or do I need to write one, then look it up using the same string (which is a bit of extra complexity on the call side and seems inefficient)?

For now this is what you'll have to do. Registering a new symbol ID needs to go through the writer so it knows to emit a symbol table that includes the new text before referencing it in the data stream. I do want to support this, but it might be a bit before I get around to implementing it.

@barries
Copy link
Author

barries commented Jan 8, 2025

Thanks! No worries, it's up and running, and working just as expected, it seems.

We'll apply an improvement if/when you happen to get around to it. Thanks again for all the great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants