Fork me on GitHub
#specter
<
2016-12-02
>
nathanmarz12:12:48

@caio I'd really like to understand the underlying issue rather than try to work around it

nathanmarz12:12:03

that type hint should be working fine

nathanmarz12:12:36

only want to resort to workaround if there's no other option

caio16:12:59

@nathanmarz I understand that, but I don’t know if it’s worth it. It’s an obscure bug (all I found was one discussion) and you’re only using a record there because it’s faster to get the resulting instance’s attributes using interop. If you’re going to choose a data structure with the sole purpose of optimizing code, why not choose one that solves it and doesn’t touch a nasty trick involving clojure’s compilation and class generation? 🙂

caio16:12:19

I think keeping this bug on the latest specter version is horrible (even worse than the lack of clarity introduced by my PR, IMHO). afaik, this is breaking at least one other open source library (xsc/invariant) and breaks a really common and simple use case for specter: recursive paths on projects that must be AOT compiled.

caio16:12:28

I am curious about what one would do to solve this bug without removing the record, but I also think there’re more important things to do other than trying to fix something without removing an optimisation “workaround”.

caio16:12:23

about the bug: it looks like the compilation generates two identical classes with names com.rpl.specter.impl.CachedPathInfo$reify__XX (they have different suffixes). it looks like what’s being said here is true: http://grokbase.com/t/gg/clojure/14c6sjq7hx/debugging-myclass-cannot-be-cast-to-myclass-exceptions#20141210sl7tnec6xz46p5zfwwnn47er2a I have no idea how to tell clojure compiler to not recompile that class/ns.

caio17:12:34

@nathanmarz my last attempt on doing this: I implemented a native class for CachedPathInfo so it improves the benchmarks. I doesn’t make it less readable and works both in java/javascript (it uses a record impl on JS). if you don’t want to use simpler DS as a workaround for solving this issue, then maybe a DS that actually (slightly) improves specter’s performance may convince you to forget about this bug 😄 the patch: https://github.com/nathanmarz/specter/compare/master…caioaao:native-impl?expand=1 benchmark logs: https://gist.github.com/caioaao/7f9c44db050aed9a5326cfd28f9efca0 you can compare it with the benchmark I did using the master branch on my pc: https://gist.github.com/caioaao/95a0cd834a08a777a184f5edf6c9bf00