Fork me on GitHub
#babashka-sci-dev
<
2022-03-18
>
borkdude08:03:33

Let's make the smallest change possible first which I guess is no 1 and let's not try to optimize too much right away. I think the overhead of starting a pod at install time should only be done if the namespace files aren't there yet. Besides that there isn't that much to win when the pods are already installed right? Maybe I don't understand exactly what the benefit of 2 is.

cap10morgan14:03:31

Yeah I’m only starting it for :describe once on install either way. And yeah, I don't think it's a big perf hit either way after that. I kind of stumbled onto no. 2 and was curious if you'd think it was better. Hard to say which is the smaller change right now, but I’ll proceed with no. 1 and see how it evolves.

borkdude14:03:17

It seems with 2 we are introducing different code-paths when the namespaces files are already there. Then we do not have to start any pods.

cap10morgan16:03:52

@borkdude Is it safe to assume that all byte arrays in the describe response are strings?

cap10morgan16:03:37

Or should I just serialize them as e.g. base64 (in the cache files)?

borkdude16:03:07

the describe response has a documented schema and you should assume that schema

cap10morgan16:03:39

OK looks like they're all strings 👍:skin-tone-2:

borkdude16:03:00

yeah, but could theoretically change in the future

borkdude16:03:08

just go by the docs

cap10morgan16:03:47

sure, definitely. the question is: do I cache the entire response or selectively pull out the currently-documented keys? and also: when I deserialize the cached data, is it important to put things that were bytes back to bytes or should I just deep-convert everything to strings and deal with both cached and uncached data the same (i.e. it's all strings)?

borkdude16:03:42

Maybe we could cache the entire bencode response as is

cap10morgan16:03:27

yeah, perhaps. loses easy human inspectability, but that's maybe a non-goal anyway

borkdude18:03:09

@cap10morgan I guess we could just not cache the describe response at all

borkdude18:03:30

if we need more info for some other feature, we could just load the pod once, extract the info and write it to a separate file

borkdude18:03:40

like the namespaces files

cap10morgan18:03:22

Trying out the raw bencode cache approach now. If it works it seems fairly elegant so far. But that's a big “if” :)

cap10morgan21:03:00

@borkdude when we run the pod at installation just for the :describe, do you think it's important to run any :shutdown op the pod has when we then shut it down again?

cap10morgan21:03:30

I'm assuming "yes"

borkdude21:03:55

it allows to gracefully shutdown, you never know what it's good for.

cap10morgan21:03:52

yeah, what I figured. just requires that I do a little more decoding of the metadata at that point in the code than I had been. no big deal though, already done. 🙂

cap10morgan21:03:41

Is there a standard file name extension for bencode data?

cap10morgan21:03:59

I guess maybe .torrent but that doesn't seem appropriate here 😆

cap10morgan21:03:46

I'll just use metadata.cache for now

borkdude21:03:39

or just describe.bnc

cap10morgan21:03:00

hey it works!

cap10morgan21:03:15

more commits to the PR coming shortly

borkdude21:03:24

🎉 🎉 🎉

cap10morgan21:03:46

uncached run: 490ms, cached run: 355ms (both with pods already installed). but that's with only one pod being required.

borkdude21:03:53

it should be just as fast as with manual load pod right?

cap10morgan21:03:49

yeah in theory. guess I should kick off the shutdown in a separate thread. no need to wait on that.

cap10morgan21:03:35

ok that's pushed and a new PR opened in babashka/pods

borkdude22:03:56

@cap10morgan Will check tomorrow

👍 1
borkdude10:03:47

@cap10morgan ok, left a couple of review comments