clintropolis commented on a change in pull request #9179: Run IntelliJ
inspections on Travis
URL: https://github.com/apache/druid/pull/9179#discussion_r367287105
##########
File path:
processing/src/main/java/org/apache/druid/segment/data/ColumnarLongs.java
##########
@@ -121,14 +122,14 @@ public boolean isNull()
@Override
public long getLong()
{
- assert NullHandling.replaceWithDefault() || !isNull();
+ Preconditions.checkState(NullHandling.replaceWithDefault() ||
!isNull());
Review comment:
This change to column gets should definitely not be done imo, the current
version with `assert` will not be evaluated at all at runtime unless asserts
are enabled with jvm command line flags, and isNull is an expensive check if
the bit is set in the bitmap, which will have already been done by anything
using the selector.
In fact, I've been meaning to investigate if they should be removed entirely
because they are still in the bytecode and could affect hotspot function
inlining which takes function size into account, since these methods will be
called from hot loops, but I haven't had the chance to investigate.
e.g. using columnar doubles
with assert:
```
public double getDouble(int);
Code:
0: getstatic #13 // Field $assertionsDisabled:Z
3: ifne 33
6: invokestatic #14 // Method
org/apache/druid/common/config/NullHandling.replaceWithDefault:()Z
9: ifne 33
12: aload_0
13: getfield #2 // Field
val$nullValueBitmap:Lorg/apache/druid/collections/bitmap/ImmutableBitmap;
16: iload_1
17: invokeinterface #19, 2 // InterfaceMethod
org/apache/druid/collections/bitmap/ImmutableBitmap.get:(I)Z
22: ifeq 33
25: new #16 // class java/lang/AssertionError
28: dup
29: invokespecial #17 // Method
java/lang/AssertionError."<init>":()V
32: athrow
33: aload_0
34: getfield #1 // Field
this$0:Lorg/apache/druid/segment/data/ColumnarDoubles;
37: iload_1
38: invokeinterface #18, 2 // InterfaceMethod
org/apache/druid/segment/data/ColumnarDoubles.get:(I)D
43: dreturn
```
no assert:
```
public double getDouble(int);
Code:
0: aload_0
1: getfield #1 // Field
this$0:Lorg/apache/druid/segment/data/ColumnarDoubles;
4: iload_1
5: invokeinterface #13, 2 // InterfaceMethod
org/apache/druid/segment/data/ColumnarDoubles.get:(I)D
10: dreturn
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
