Fix NifTaggedEnum derived Encoder impl for named-field variants

[dylanburati]

Hi 👋,

On rustler 0.28.0, I get an error in the generated code from enums like this:

#[derive(NifTaggedEnum)]
enum TaggedEnum {
    Numbers { unit: Option<String>, values: Vec<f64> }
}
/*
error[E0308]: mismatched types
  --> src/types.rs:85:29
   |
85 |                     &[unit, values],
   |                             ^^^^^^ expected `&Option<String>`, found `&Vec<f64>`
   |
   = note: expected reference `&std::option::Option<std::string::String>`
              found reference `&Vec<f64>`
*/

The slice was being passed to map_from_arrays. I looked at NifMap's equivalent gen_named_encoder and copied the map_put strategy, which worked.

If performance is a concern, I think a set of map_from(env, k1, v1, ...) functions for small map sizes could accomplish the same thing but still use enif_make_map_from_arrays.

[filmor]

Hmm, do you have an explanation what's wrong with the previous code? map_from_arrays should be more efficient, so we should probably try to fix that code path.

[dylanburati]

I don't think you can make a slice for the values: &[impl Encoder] parameter with more than one type, at least without boxing each value. It gives the same error for me even if I just use u32 and i32.

I agree on the efficiency part - I'll see if I can figure out the alternate option of map_from(env, k1, v1, ...) later today.

[dylanburati]

This StackOverflow post has some better explanations of that type of error.

The solution they got was basically values: &[&dyn Encoder] but I think dyn would make the Encoder::encode() method of each element resolve at runtime.

[dylanburati]

I made some local changes in order to get the NifStruct benchmark to run using different near-copies of map_from_arrays.

• map_put (baseline): 851k iters/sec decode, 270k roundtrip, 1/(1/270k - 1/851k) = 395k iters/sec encode
• map_from_iterables: 840k, 569k, 1.764M
  • Signature uses trait with implementations on tuples size 1-27
  • Call is same as map_from_arrays but with the tuples in place of the arrays
• map_from_dyn_arrays: 890k, 634k, 2.204M
  • Signature uses &[&dyn Encoder]
  • Call is same as map_from_arrays, but every array element needs an & in front of it
• map_from_term_arrays: 866k, 635k, 2.380M
  • Signature uses &[Term<'a>]
  • Caller needs to encode with the Env<'a> themselves

The struct being encoded is just 26 i64 values. I think the overhead of dyn might show up more prominently if there were more Encoder types around. Either way, I think map_from_term_arrays makes sense here.

@filmor Do you have a preference?

[filmor]

I'd prefer map_from_term_arrays, this is what I would have suggested as well :)

Thank you very much for the thorough testing and evaluation!