search

techascent / tech.datatype

Efficient numerics for the jvm
Eclipse Public License 1.0
83 stars 8 forks source link

fix: reflection warnings on nio_buffer #25

Closed rschmukler closed 4 years ago

rschmukler commented 4 years ago

Fixes reflection warnings on the nio_buffer namespace by adding a typecast/datatype->value macro which performs the correct type annotation based on the datatype keyword.

I looked at the implementation of casting/cast and was a bit surprised that the compiler didn't manage to track the type through things.

Turns out annotating variables with type data using macros is a bit tricky, but this seems to take care of the reflection warnings.

cnuernber commented 4 years ago

This is helpful and I will take a look at this soon. I thought I caught everything with my latest lein check pathways but we appeared to have missed it.

(casting/datatype->unchecked-cast :unknown ~datatype ~value) is what you want here. That will cast things correctly given your starting object type (unknown in this case) and your ending object type (known in this case) and the value. It boils down the primitive-math primitives.

cnuernber commented 4 years ago

I guess also looking at this if the nio buffer is backed by a primitive array then Arrays/fill may be a better choice and then failing that this loop.

rschmukler commented 4 years ago

Thanks for the reply @cnuernber - I'm afraid I don't have enough familiarity with the code base to do the primitive array filling, but I did update the PR to use (casting/datatype->unchecked-cast-fn :unknown ~datatype ~value) and got rid of my redundant macro.

I'm also unsure whether the first cast is necessary anymore. I removed it and the tests are still passing, but please let me know if you'd like me to re-add it.

cnuernber commented 4 years ago

You got it. This looks great and I totally understand. Will profile this a bit and see if arrays/fill is faster or not and then go from there.

I also need to make memset/memcopy direct-mapped functions as the jna overhead on dynamic fns is pretty intense. In any case, this is a solid change, thanks.

cnuernber commented 4 years ago

looking into Arrays/fill now:


tech.v2.datatype.nio-buffer> (crit/quick-bench (Arrays/fill array-data 0 100000 -1.0))
Evaluation count : 8172 in 6 samples of 1362 calls.
             Execution time mean : 76.566637 µs
    Execution time std-deviation : 2.271296 µs
   Execution time lower quantile : 73.802201 µs ( 2.5%)
   Execution time upper quantile : 79.064141 µs (97.5%)
                   Overhead used : 10.374248 ns
nil
tech.v2.datatype.nio-buffer> (def buf-data (DoubleBuffer/wrap array-data))
#'tech.v2.datatype.nio-buffer/buf-data
tech.v2.datatype.nio-buffer> (crit/quick-bench 
                              (parallel-for/parallel-for 
                               idx 100000
                               (.put ^DoubleBuffer buf-data idx -1.0)))
Evaluation count : 966 in 6 samples of 161 calls.
             Execution time mean : 613.808717 µs
    Execution time std-deviation : 13.732219 µs
   Execution time lower quantile : 596.789547 µs ( 2.5%)
   Execution time upper quantile : 626.654790 µs (97.5%)
                   Overhead used : 10.374248 ns
nil

Somehow Arrays/fill is 10X faster than a parallelized loop for setting value :-).

cnuernber commented 4 years ago

New release out containing this and other fixes.

rschmukler commented 4 years ago

Woo! That's awesome. 10x out of nowhere isn't bad!