On Mon Sep 14, 2026 at 4:04 PM JST, John Hubbard wrote:
> On 9/13/26 11:55 PM, Eliot Courtney wrote:
>> On Mon Sep 14, 2026 at 12:46 PM JST, Alexandre Courbot wrote:
>>> On Thu Aug 27, 2026 at 11:12 PM JST, Eliot Courtney wrote:
> ...
>>>> + fn visit(
>>>> + &mut self,
>>>> + key: $crate::gsp::nvkv::KeyId,
>>>> + index: $crate::gsp::nvkv::Index,
>>>> + value: $crate::gsp::nvkv::DecoderValue<'_>,
>>>> + ) -> ::kernel::error::Result<bool> {
>>>> + Ok(false
>>>> + $( || $crate::gsp::nvkv::Schema::visit(&mut
>>>> self.$field, key, index, value)? )*)
>>>
>>> Mmm looks like this is going to be `O(n)` with `n` being the number of
>>> fields?
>>>
>>> This is ok for a first implementation but eventually I hope we can
>>> switch to a more efficient dispatch.
>>
>> I thought quite a bit about this while writing this code, since we need
>> the escape hatch to imperative decode (custom Schema impl basically). To
>> be able to get it down to a match on the key, we need to know ahead of
>> time which keys a Schema will consume. That duplicates the info from the
>> visit() implementation.
>>
>> I thought up a few methods but it's unclear to me which one is best, so
>> I just left it for now. Please LMK if you think this is urgent, I can
>> try in a follow up to improve this. Here are my ideas (when I say O(1)
>> lookup I mean modulo how the compiler decides to do it with the set of
>> key IDs it gets):
>>
>> 1. current code - just visit()
>> pros: key source of truth not duplicates
>> cons: O(field) visit as you say
>>
>
> Something about the visit pattern has always concerned me, and I think
> it has to do with the fact that people have been told to use it in
> all kinds of situations. But it's really only ideal for a few situations,
> such as updated a bunch of objects on a display, for example.
>
> It's not a good fit here IMHO.
>
> Is it hard to do one of the other choices, up front? If it's easy,
> it would be nice to skip this visit entirely. :)
>
> thanks,
It's not too hard to do one of #2-4 now, it's just a bit of extra
complication which is why I left it out. The performance gain is kinda
minimal since this doesn't happen in a hot path.
That said, all the options I mention here still use `visit`. #2-4 are
all about adding extra info to help avoid having to call `visit` as much
+ let the compiler have more info so it can generate a jump table etc.
That said, if you have an alternate approach, happy to hear it.
