Fix #1775: use optimized CqlVector<Float> codec to improve performance #1801
Conversation
|
So far so good: return direction (Vector results from driver) now works. |
tatu-at-datastax
changed the title
| Iterator<String> strIterator = strIterable.iterator(); | ||
| for (int i = 0; i < rv.length; ++i) { | ||
| String strVal = strIterator.next(); | ||
| // TODO: String.isBlank() should be included here but it's only available with Java11+ |
There was a problem hiding this comment.
Since we are using Java 21, why don't we use String.isBlank() here?
There was a problem hiding this comment.
Code was first written for Java CQL driver (by Brett). I can change that, thanks.
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ |
There was a problem hiding this comment.
Why do we need this?
There was a problem hiding this comment.
Similarly just copied over, we don't. Can remove.
| public class SubtypeOnlyFloatVectorTest { | ||
|
|
||
| @Test | ||
| public void should_find_subtype_only_codec_regardless_of_size() { |
There was a problem hiding this comment.
Just curious, why do we use the snake case but not the camel case? Is it because the name is too long?
There was a problem hiding this comment.
Same as earlier, something Java driver probably does (I copied over as-is).
Not something we should do.
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ |
There was a problem hiding this comment.
Same question as the above
| } | ||
|
|
||
| @Test | ||
| public void should_encode() { |
There was a problem hiding this comment.
Why are all the methods in this class using snake case?
| throw new IllegalArgumentException( | ||
| String.format("Input ByteBuffer should have a multiple of %d bytes", ELEMENT_SIZE)); |
There was a problem hiding this comment.
Not sure how common this exception is. If this is a common error, should we use Error V2 instead of IllegalArgumentException? Have the same question for other IllegalArgumentExceptions. Or all these IllegalArgumentExceptions will be caught eventually and converted to our own errors? I see there is caught in VectorCodec and convert it to ToCQLCodecException, but not sure if it covers all the cases.
There was a problem hiding this comment.
This codec is used by Java CQL Driver, it's not a Data API codec. But I think code we use for java driver will have to catch exceptions anyway. Problem itself should not be possible to occur by any external calls; it is more an assertion (and as such not sure there is a way to easily test).
What this PR does:
Will use optimized CQL driver codec for
Vector<Float>to reduce extra memory allocations, improve performance of reading and writing CQL Float vectors (problem found by profiling).NOTE: optimized codec is from here:
https://github.com/absurdfarce/cassandra-java-driver/tree/subtype-only-vector-codec
Which issue(s) this PR fixes:
Fixes #1775
Checklist