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

Compile string constants into unique.Handle[string] #1018

Open
3 tasks done
pohly opened this issue Sep 10, 2024 · 2 comments
Open
3 tasks done

Compile string constants into unique.Handle[string] #1018

pohly opened this issue Sep 10, 2024 · 2 comments

Comments

@pohly
Copy link

pohly commented Sep 10, 2024

Feature request checklist

  • There are no issues that match the desired change
  • The change is large enough it can't be addressed with a simple Pull Request
  • If this is a bug, please file a Bug Report.

Change

Go 1.23 added support for internalizing strings with a standard API (https://pkg.go.dev/unique). When compiling an expression, the AST could store string constants as unique.Handle[string]. Conversion to a normal string could happen on the fly.

Example

I'm implementing a custom map with strings as key:

func (d deviceAttributeDomains) Contains(index ref.Val) ref.Val {
	strKey, ok := index.ConvertToType(types.UniqueStringType).Value().(unique.Handle[string])
	if !ok {
		return types.False
	}
	_, ok = d[strKey]
	return types.Bool(ok)
}

With unique strings, the map becomes more efficient.

Alternatives considered

A normal string extracted from CEL could be turned into a handle before looking it up in a map which uses internalized strings. But that causes overhead during each evaluation which could be moved into the one-time compilation.

@TristonianJones
Copy link
Collaborator

It's an interesting feature. I imagine it'd be most useful for literals and the AST. Is that what you have in mind as well? Or something else?

@pohly
Copy link
Author

pohly commented Sep 11, 2024

Yes, exactly that.

In my experience, unique.Make can be expensive. But if it is done once and the result is used often, it pays off. Using internalized strings might have to be a compile option because the user should decide about this tradeoff.

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