zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2684967526
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java:
##########
@@ -179,6 +180,20 @@ TreeSet.class, new BinaryTreeSetWriteReplacer()
/** FNV1 hash prime. */
private static final int FNV1_PRIME = 0x01000193;
+ /** */
+ public static final BinariesFactory binariesFactory;
+
+ static {
+ Iterator<BinariesFactory> factories =
CommonUtils.loadService(BinariesFactory.class).iterator();
+
+ A.ensure(
+ factories.hasNext(),
+ "Implementation for BinariesFactory service not found. Please add
ignite-binary-impl to classpath"
Review Comment:
```suggestion
"Implementation for " + BinariesFactory.class.getSimpleName() +
" service not found. Please add ignite-binary-impl to classpath"
```
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java:
##########
@@ -18,911 +18,45 @@
package org.apache.ignite.internal.binary;
import java.lang.reflect.Field;
-import java.math.BigDecimal;
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.Collection;
-import java.util.Date;
-import java.util.Map;
-import java.util.UUID;
-import org.apache.ignite.binary.BinaryObjectException;
-import org.apache.ignite.internal.UnregisteredBinaryTypeException;
-import org.apache.ignite.internal.UnregisteredClassException;
-import org.apache.ignite.internal.util.CommonUtils;
-import org.apache.ignite.internal.util.GridUnsafe;
-import org.apache.ignite.internal.util.typedef.F;
-import org.apache.ignite.internal.util.typedef.internal.S;
/**
* Field accessor to speedup access.
Review Comment:
I suppose - after accessors are removed this class is not an accessor at all
but pojo for holding some params and probably can be renamed, what do you think
?
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.
+ */
+
+package org.apache.ignite.internal.binary;
+
+import java.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal
object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in,
ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal
object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal
object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean skipHdrCheck,
+ boolean forUnmarshal);
+
+ /**
+ * @param ctx Context.
+ * @param failIfUnregistered Flag to fail while writing object of
unregistered type.
+ * @param typeId Type id.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, boolean
failIfUnregistered, int typeId);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out,
BinaryWriterSchemaHolder schema);
+
+ /**
+ * Create accessor for the field.
+ *
+ * @param field Field.
+ * @param id FIeld ID.
Review Comment:
typo
##########
modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java:
##########
@@ -1586,4 +1567,298 @@ private void clearState(boolean clearHandler) {
if (clearHandler)
this.handles = null;
}
+
+ /** {@inheritDoc} */
+ @Override public void writeField(Object obj, BinaryFieldAccessor fld)
throws BinaryObjectException {
+ writeFieldIdNoSchemaUpdate(fld.id);
+
+ switch (fld.mode) {
+ case P_BYTE:
+ writeByteFieldPrimitive(GridUnsafe.getByteField(obj,
fld.offset));
+
+ break;
+
+ case P_BOOLEAN:
+ writeBooleanFieldPrimitive(GridUnsafe.getBooleanField(obj,
fld.offset));
+
+ break;
+
+ case P_SHORT:
+ writeShortFieldPrimitive(GridUnsafe.getShortField(obj,
fld.offset));
+
+ break;
+
+ case P_CHAR:
+ writeCharFieldPrimitive(GridUnsafe.getCharField(obj,
fld.offset));
+
+ break;
+
+ case P_INT:
+ writeIntFieldPrimitive(GridUnsafe.getIntField(obj,
fld.offset));
+
+ break;
+
+ case P_LONG:
+ writeLongFieldPrimitive(GridUnsafe.getLongField(obj,
fld.offset));
+
+ break;
+
+ case P_FLOAT:
+ writeFloatFieldPrimitive(GridUnsafe.getFloatField(obj,
fld.offset));
+
+ break;
+
+ case P_DOUBLE:
+ writeDoubleFieldPrimitive(GridUnsafe.getDoubleField(obj,
fld.offset));
+
+ break;
+
+ case BYTE:
+ case BOOLEAN:
+ case SHORT:
+ case CHAR:
+ case INT:
+ case LONG:
+ case FLOAT:
+ case DOUBLE:
+ case DECIMAL:
+ case STRING:
+ case UUID:
+ case DATE:
+ case TIMESTAMP:
+ case TIME:
+ case BYTE_ARR:
+ case SHORT_ARR:
+ case INT_ARR:
+ case LONG_ARR:
+ case FLOAT_ARR:
+ case DOUBLE_ARR:
+ case CHAR_ARR:
+ case BOOLEAN_ARR:
+ case DECIMAL_ARR:
+ case STRING_ARR:
+ case UUID_ARR:
+ case DATE_ARR:
+ case TIMESTAMP_ARR:
+ case TIME_ARR:
+ case ENUM_ARR:
+ case OBJECT_ARR:
+ case BINARY_OBJ:
+ case BINARY:
+ default:
+ assert obj != null;
+
+ Object val;
+
+ try {
+ val = fld.field.get(obj);
+ }
+ catch (IllegalAccessException e) {
+ throw new BinaryObjectException("Failed to get value for
field: " + fld.field, e);
+ }
+
+ switch (mode(fld, val)) {
+ case BYTE:
+ writeByteField((Byte)val);
+
+ break;
+
+ case SHORT:
+ writeShortField((Short)val);
+
+ break;
+
+ case INT:
+ writeIntField((Integer)val);
+
+ break;
+
+ case LONG:
+ writeLongField((Long)val);
+
+ break;
+
+ case FLOAT:
+ writeFloatField((Float)val);
+
+ break;
+
+ case DOUBLE:
+ writeDoubleField((Double)val);
+
+ break;
+
+ case CHAR:
+ writeCharField((Character)val);
+
+ break;
+
+ case BOOLEAN:
+ writeBooleanField((Boolean)val);
+
+ break;
+
+ case DECIMAL:
+ writeDecimal((BigDecimal)val);
+
+ break;
+
+ case STRING:
+ writeString((String)val);
+
+ break;
+
+ case UUID:
+ writeUuid((UUID)val);
+
+ break;
+
+ case DATE:
+ writeDate((Date)val);
+
+ break;
+
+ case TIMESTAMP:
+ writeTimestamp((Timestamp)val);
+
+ break;
+
+ case TIME:
+ writeTime((Time)val);
+
+ break;
+
+ case BYTE_ARR:
+ writeByteArray((byte[])val);
+
+ break;
+
+ case SHORT_ARR:
+ writeShortArray((short[])val);
+
+ break;
+
+ case INT_ARR:
+ writeIntArray((int[])val);
+
+ break;
+
+ case LONG_ARR:
+ writeLongArray((long[])val);
+
+ break;
+
+ case FLOAT_ARR:
+ writeFloatArray((float[])val);
+
+ break;
+
+ case DOUBLE_ARR:
+ writeDoubleArray((double[])val);
+
+ break;
+
+ case CHAR_ARR:
+ writeCharArray((char[])val);
+
+ break;
+
+ case BOOLEAN_ARR:
+ writeBooleanArray((boolean[])val);
+
+ break;
+
+ case DECIMAL_ARR:
+ writeDecimalArray((BigDecimal[])val);
+
+ break;
+
+ case STRING_ARR:
+ writeStringArray((String[])val);
+
+ break;
+
+ case UUID_ARR:
+ writeUuidArray((UUID[])val);
+
+ break;
+
+ case DATE_ARR:
+ writeDateArray((Date[])val);
+
+ break;
+
+ case TIMESTAMP_ARR:
+ writeTimestampArray((Timestamp[])val);
+
+ break;
+
+ case TIME_ARR:
+ writeTimeArray((Time[])val);
+
+ break;
+
+ case OBJECT_ARR:
+ writeObjectArray((Object[])val);
+
+ break;
+
+ case COL:
+ writeCollection((Collection<?>)val);
+
+ break;
+
+ case MAP:
+ writeMap((Map<?, ?>)val);
+
+ break;
+
+ case BINARY_OBJ:
+ writeBinaryObject((BinaryObjectImpl)val);
+
+ break;
+
+ case ENUM:
+ writeEnum((Enum<?>)val);
+
+ break;
+
+ case BINARY_ENUM:
+ writeBinaryEnum((BinaryEnumObjectImpl)val);
+
+ break;
+
+ case ENUM_ARR:
+ doWriteEnumArray((Object[])val);
+
+ break;
+
+ case BINARY:
+ case OBJECT:
+ case PROXY:
+ writeObject(val);
+
+ break;
+
+ case CLASS:
+ writeClass((Class)val);
+
+ break;
+
+ default:
+ assert false : "Invalid mode: " + fld.mode;
Review Comment:
probably AssertionError need to be thrown from here ? It looks like
unexpected behavior, wdyt ?
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java:
##########
@@ -781,7 +781,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws
BinaryObjectException {
break;
case BINARY_ENUM:
- writer.writeBinaryEnum((BinaryEnumObjectImpl)obj);
+ writer.writeBinaryEnum((BinaryObjectEx)obj);
Review Comment:
whats the purpose in such a change ? writeBinaryEnum implementation still
invalidate that "val instanceof BinaryEnumObjectImpl" also it can broke smth if
assertions are disabled
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java:
##########
@@ -18,911 +18,45 @@
package org.apache.ignite.internal.binary;
import java.lang.reflect.Field;
-import java.math.BigDecimal;
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.Collection;
-import java.util.Date;
-import java.util.Map;
-import java.util.UUID;
-import org.apache.ignite.binary.BinaryObjectException;
-import org.apache.ignite.internal.UnregisteredBinaryTypeException;
-import org.apache.ignite.internal.UnregisteredClassException;
-import org.apache.ignite.internal.util.CommonUtils;
-import org.apache.ignite.internal.util.GridUnsafe;
-import org.apache.ignite.internal.util.typedef.F;
-import org.apache.ignite.internal.util.typedef.internal.S;
/**
* Field accessor to speedup access.
*/
-abstract class BinaryFieldAccessor {
+class BinaryFieldAccessor {
/** Field ID. */
- protected final int id;
+ final int id;
/** Field name */
- protected final String name;
+ final String name;
/** Mode. */
- protected final BinaryWriteMode mode;
+ final BinaryWriteMode mode;
- /**
- * Create accessor for the field.
- *
- * @param field Field.
- * @param id FIeld ID.
- * @return Accessor.
- */
- public static BinaryFieldAccessor create(Field field, int id) {
- BinaryWriteMode mode = BinaryUtils.mode(field.getType());
-
- switch (mode) {
- case P_BYTE:
- return new BytePrimitiveAccessor(field, id);
-
- case P_BOOLEAN:
- return new BooleanPrimitiveAccessor(field, id);
-
- case P_SHORT:
- return new ShortPrimitiveAccessor(field, id);
-
- case P_CHAR:
- return new CharPrimitiveAccessor(field, id);
-
- case P_INT:
- return new IntPrimitiveAccessor(field, id);
-
- case P_LONG:
- return new LongPrimitiveAccessor(field, id);
+ /** Offset. Used for primitive fields, only. */
+ final long offset;
- case P_FLOAT:
- return new FloatPrimitiveAccessor(field, id);
+ /** Target field. */
+ final Field field;
- case P_DOUBLE:
- return new DoublePrimitiveAccessor(field, id);
-
- case BYTE:
- case BOOLEAN:
- case SHORT:
- case CHAR:
- case INT:
- case LONG:
- case FLOAT:
- case DOUBLE:
- case DECIMAL:
- case STRING:
- case UUID:
- case DATE:
- case TIMESTAMP:
- case TIME:
- case BYTE_ARR:
- case SHORT_ARR:
- case INT_ARR:
- case LONG_ARR:
- case FLOAT_ARR:
- case DOUBLE_ARR:
- case CHAR_ARR:
- case BOOLEAN_ARR:
- case DECIMAL_ARR:
- case STRING_ARR:
- case UUID_ARR:
- case DATE_ARR:
- case TIMESTAMP_ARR:
- case TIME_ARR:
- case ENUM_ARR:
- case OBJECT_ARR:
- case BINARY_OBJ:
- case BINARY:
- return new DefaultFinalClassAccessor(field, id, mode, false);
-
- default:
- return new DefaultFinalClassAccessor(field, id, mode,
!CommonUtils.isFinal(field.getType()));
- }
- }
+ /** Dynamic accessor flag. */
+ final boolean dynamic;
/**
* Protected constructor.
*
* @param id Field ID.
* @param mode Mode;
*/
- protected BinaryFieldAccessor(Field field, int id, BinaryWriteMode mode) {
+ protected BinaryFieldAccessor(Field field, int id, BinaryWriteMode mode,
long offset, boolean dynamic) {
Review Comment:
plz fix java doc, append missing params
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryReaderEx.java:
##########
@@ -85,4 +85,11 @@ public interface BinaryReaderEx extends BinaryReader,
BinaryRawReader, BinaryRea
* @return Offset.
*/
public boolean findFieldByName(String name);
+
+ /**
+ * Get or create object schema.
+ *
+ * @return Schema.
+ */
+ public BinarySchema getOrCreateSchema();
Review Comment:
BinarySchema has package private visibility is it ok to have such approach
in interface ? probably it need to be changed with "public" ? wdyt ?
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryWriterEx.java:
##########
@@ -131,6 +132,11 @@ public interface BinaryWriterEx extends BinaryWriter,
BinaryRawWriter, ObjectOut
*/
public int schemaId();
+ /**
+ * @param schemaId Schema ID.
+ */
+ public void schemaId(int schemaId);
Review Comment:
i understand why you need it but just for defence from potential problems
probably it need to be appended sode additional assert check for schemaId
change invariants ?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
