Unnecessary boxing of return from set!

[pete windle]

Hey, I'm trying to work on some performance sensitive code using Clojure together with the Carrotsearch HPPC library. I've come up against a weird behaviour of set! in conjunction with primitive maths.

This example is a toy problem not a production problem, but there are things I might not be harder to do at work w/Clojure.

I have a com.carrotsearch.hppc.LongLongHashMap and I wish to sum the contents of the map. They provide a com.carrotsearch.hppc.LongLongProcedure where an apply method is called for each k, v.

Thence:

(defprotocol ValueRetriever
  (get-value [this ^LongLongHashMap memory]))

(deftype ValueAdder [^{:unsynchronized-mutable true} ^long total]
  LongLongProcedure
  (^void apply [this ^long k ^long v]
   (set! total (unchecked-add total v)))
  ValueRetriever
  (get-value [this memory] (set! total 0) (.forEach memory this) total))

To a first approximation all of the time spent summing the map is in the apply method as expected, however when I profile it with YourKit every sample taken is actually in clojure.lang.Numbers.num. Using the extremely handy clj-java-decompiler library I can try to see what's happening, and it looks like we're attempting to box the return value from set!

    public void apply(final long k, final long n) {
        Numbers.num(this.total += n);
    }

Is there some technique I can use to stop the return value from set! being boxed (before the box is discarded to the void)?

I do have real use cases where a rather tight aggregation loop will be called for many millions of values and I'd prefer not to incur this cost.

Workaround is obviously to write the aggregators in Java but that's strongly not preferred, at the point I'm mixing modes I might as well write the whole core in Java.

Cheers,

Pete Windle

[Erik Assum]

This reminds me of a bug that I believe Ghadi fixed for 1.11, 

https://clojure.atlassian.net/browse/CLJ-2621

Don’t know if that helps much, though. 

Erik. 

-- 

i farta

15. mai 2022 kl. 14:11 skrev pete windle <goo...@pete23.com>:

Hey, I'm trying to work on some performance sensitive code using Clojure together with the Carrotsearch HPPC library. I've come up against a weird behaviour of set! in conjunction with primitive maths.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/clojure/d9eccd28-3db3-4e6f-88f9-7a4106fc05aan%40googlegroups.com.

[pete windle]

Definitely in the same wheelhouse, but upgrading to 1.11.1 shows the same behaviour - as expected, because this is not an InstanceMethodExpr.

https://github.com/clojure/clojure/blame/master/src/jvm/clojure/lang/Compiler.java#L1831-L1842 is the logic for StaticMethodExpr.

AssignExpr hands off to the AssignableExpr target which in my case is the InstanceFieldExpr(?)

Takes us to here I guess: https://github.com/clojure/clojure/blame/master/src/jvm/clojure/lang/Compiler.java#L1237-L1259

Unless I roll my sleeves up and debug into this to check exactly where the boxing is being emitted not sure I will get further, feels like I'm deeper in the innards of Compiler.java than I'm qualified for without more thought and reading.

Thanks though, helpful pointer!

Pete Windle

[pete windle]

I wondered if this would work if I handed the result off to a helper class

public class VoidHelper {
    public static void voidLong(long x) {}
}

If this somehow improved the code path, with inlining etc it would be basically free.

...but the generated code gets worse.

    public void apply(final long k, final long n) {

        this.total += n;
        final long longCast = RT.longCast(Numbers.num(this.total));
        this = null;
        VoidHelper.voidLong(longCast);
    }

total is long, but we box it with Numbers.num and then RT.longCast it before calling voidLong. Hmmm. Feels like the knowledge that it's returning a primitive type is missing.

Thanks,

Pete Windle

[Alex Miller]

I don't think that void type hint is going to do anything there.  The deftype impl of apply here will (has to by Java requirements) return void here. There is a gap here I think where the return gets needlessly boxed. You might try just putting a nil expr after the set! as a workaround.

In any case, we should definitely get a ticket filed and track this down.

[pete windle]

Hi Alex,

• the ^void is necessary to get the method signature right for it to compile at all
  
• the nil workaround "works" in that decompilation shows the method body with the addition only, no cast - thank you
• ...but makes a smaller difference to my overall execution time:-)
• lesson learned (remembered): use asynchronous profiling to avoid safepoint bias
• when I do that without the workaround it's clear that the addition is 50% of the observable work in that method, the Numbers.num Long.valueOf() calls the other 50% - so eliminating the cast doesn't make everything free

Thanks,

Pete Windle