sparkling icon indicating copy to clipboard operation
sparkling copied to clipboard

Reflection warning when using destructuring sugar

Open vvvvalvalval opened this issue 6 years ago • 2 comments

The problem

I'm observing some Reflection Warnings at the REPL when calling the s-de/fn macro, e.g

(do
  (set! *warn-on-reflection* true)

  (s-de/fn
    [(x ?y)]
    (+ x (or y 0))))
;Reflection warning, /Users/val/projects/.../data_sources.clj:48:3 - reference to field _1 can't be resolved.
;Reflection warning, /Users/val/projects/.../data_sources.clj:48:3 - reference to field _2 can't be resolved.
;Reflection warning, /Users/val/projects/.../data_sources.clj:48:3 - reference to field orNull can't be resolved.
=>
#object[linnaeus.imports.spark_scripts.data_sources$eval25833$fn__25834
        0x49b1f844
        "linnaeus.imports.spark_scripts.data_sources$eval25833$fn__25834@49b1f844"]

Why I think it's important

Method/field calls done via reflection have very bad performance, which tends to matter in the context of sparkling IMO - the destructuring code will tend to be called for each element to process.

Investigating the cause

Looking at the source code for sparkling.destructuring, I suspect this is because the TupleN class on which to call ._1, ._2 etc. can't be inferred statically (which is understandable in the context of Clojure).

Proposed solutions

For tuple destructuring

I suggest giving the caller the opportunity to provide a type-hint using meta-data on the list form, e.g

(s-de/fn
  [^{::s-de/type-hint Scala.Tuple2} (x ?y)]
  (+ x (or y 0)))

Or maybe the higher-level:

(s-de/fn
  [^{::s-de/tuple-arity 2} (x ?y)]
  (+ x (or y 0)))

From there, the macro could emit a type hint itself, or dispatch to a function that has a type hint etc.

This is not super readable, so we may complement this by:

  1. Having a single boolean flag and let the macro figure the Tuple arity from the length of the list:
(s-de/fn
  [^::s-de/type-hinted (x ?y)]
  (+ x (or y 0)))
  1. Having this type-hint on the argument vector itself, specifying that all nested tuples should be automatically type-hinted:
(s-de/fn
  ^::s-de/type-hinted [(x ?y (u w z))]
  (+ x (or y 0)))

For Optional Destructuring

Actually, this one can be inferred statically - all it takes is to have the emitted code use an Optional type hint, e.g by emitting a call to optional-or-nil.

Happy to provide a PR if needed! Keep up the great work.

vvvvalvalval avatar Feb 16 '19 08:02 vvvvalvalval

Hi @vvvvalvalval Thanks for that input. Currently I'm really busy finishing up another (company) project, but I'll look into this.

Happy hacking,

Chris

chrisbetz avatar Feb 19 '19 20:02 chrisbetz

Hi @chrisbetz , just following up with this. Let me know if I can help.

vvvvalvalval avatar Apr 16 '19 12:04 vvvvalvalval