Cache import value via thunk

[sparkprime]

@huggsboson

[huggsboson]

sounds like it doesn't give much benefit from a perf perspective for my use-case. I can't tell if this helps or harms your internal invariants / UX on stack traces.

[sparkprime]

@fare

[sparkprime]

I could modify it to remove the thunk <import> bit very easily. I just did that as a knee-jerk because some other code that uses thunks (array elements, etc) did something similar.

[huggsboson]

does thunk mean the expression gets reevaluated each time or does it do
some caching of values?

On Oct 5, 2016 9:05 AM, "Dave Cunningham" [email protected] wrote:

I could modify it to remove the thunk bit very easily. I just did that as
a knee-jerk because some other code that uses thunks (array elements, etc)
did something similar.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#241 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADUzzBbL-PmFhooa725wWpw3eg7ulKaaks5qw8qAgaJpZM4KOOUR
.

[fare]

Am I correctly understanding that your cache key is the pair of the filename and file contents returned by the import callback, as opposed to e.g. simply the unexpanded string passed to the callback?

If so, this looks good to me. (At least, a definitive improvement, that would resolve the exponential blowout issue with function libraries)

Of course, for a better long term solution, you might still want to modify the import callback protocol, so you don't need to include the file contents in the key, only the resolved import filename. Thus, there would be two callback functions, one to resolve the filename, the other to get the file contents. And you could then cache merely on the resolved filename.

[sparkprime]

@huggsboson A thunk is a heap object that caches the value when it's resolved. It's used to implement the laziness (i.e. when executing local x = foo; bar, x is bound to a thunk, then bar is executed and if x is used in bar then the thunk is evaluated and the second time x is used in bar then the cached value is used). Using the thunk in this context is just an easy way to get the same behavior. It's also used to make arrays lazy (i.e. arrays are actually arrays of thunks).

@fare Imports (both importstr and import) always cached the file content (array of bytes). A previous PR modified the import behavior to cache the parsed & transformed AST in addition to the array of bytes to save some time on subsequent imports. This PR changes this to cache the actual value after execution. The key of the cache is the resolved path, from the perspective of the working directory of the native Jsonnet executable. So it will only get executed twice if you use a symlink or something to create two logical files. For custom import callbacks you have to resolve the path yourself so yes I think what you're saying is correct in that case.

[sparkprime]

@fare Out of paranoia I checked the code. Turns out that paranoia was justified: The cache key is a pair of (dir, path) so it would execute it twice if you import "foo.libsonnet" and "bar/../foo.libsonnet". I agree that's not what we want.