sergey-chugunov-1985 commented on code in PR #12266:
URL: https://github.com/apache/ignite/pull/12266#discussion_r2272887836
##########
modules/commons/src/main/java/org/apache/ignite/internal/util/CommonUtils.java:
##########
@@ -455,4 +491,253 @@ public static void writeUuid(DataOutput out, UUID uid)
throws IOException {
return null;
}
+
+ /**
+ * Get current Ignite name.
+ *
+ * @return Current Ignite name.
+ */
+ @Nullable public static String getCurrentIgniteName() {
+ return LOC_IGNITE_NAME.get();
+ }
+
+ /**
+ * Check if current Ignite name is set.
+ *
+ * @param name Name to check.
+ * @return {@code True} if set.
+ */
+ @SuppressWarnings("StringEquality")
+ public static boolean isCurrentIgniteNameSet(@Nullable String name) {
+ return name != LOC_IGNITE_NAME_EMPTY;
+ }
+
+ /**
+ * Set current Ignite name.
+ *
+ * @param newName New name.
+ * @return Old name.
+ */
+ @SuppressWarnings("StringEquality")
+ @Nullable public static String setCurrentIgniteName(@Nullable String
newName) {
+ String oldName = LOC_IGNITE_NAME.get();
+
+ if (oldName != newName)
+ LOC_IGNITE_NAME.set(newName);
+
+ return oldName;
+ }
+
+ /**
+ * Restore old Ignite name.
+ *
+ * @param oldName Old name.
+ * @param curName Current name.
+ */
+ @SuppressWarnings("StringEquality")
+ public static void restoreOldIgniteName(@Nullable String oldName,
@Nullable String curName) {
+ if (oldName != curName)
+ LOC_IGNITE_NAME.set(oldName);
+ }
+
+ /**
+ * @return Class loader used to load Ignite itself.
+ */
+ public static ClassLoader gridClassLoader() {
+ return gridClassLoader;
+ }
+
+ /**
+ * @param ldr Custom class loader.
+ * @param cfgLdr Class loader from config.
+ * @return ClassLoader passed as param in case it is not null or cfgLdr
in case it is not null or ClassLoader used to start Ignite.
+ */
+ public static ClassLoader resolveClassLoader(@Nullable ClassLoader ldr,
@Nullable ClassLoader cfgLdr) {
+ return (ldr != null && ldr != gridClassLoader)
+ ? ldr
+ : cfgLdr != null
+ ? cfgLdr
+ : gridClassLoader;
+ }
+
+ /**
+ * Tests whether or not given class is loadable provided class loader.
Review Comment:
Lets fix some javadocs as we touch this code.
`Tests whether given class is loadable by provided class loader.` is better
##########
modules/binary/api/src/main/java/org/apache/ignite/marshaller/MarshallerExclusions.java:
##########
@@ -114,18 +63,12 @@ private MarshallerExclusions() {
private static boolean isExcluded0(Class<?> cls) {
assert cls != null;
- final Class<?>[] inc = INCL_CLASSES;
-
- // NOTE: don't use foreach for performance reasons.
- for (int i = 0; i < inc.length; i++)
- if (inc[i].isAssignableFrom(cls))
+ for (Class<?> inclCls : INCL_CLASSES)
Review Comment:
Do we have an idea of how often `isExcluded0` is called during normal
operations? Is it possible that this code is indeed on a hot path thus
allocating an iterator would indeed create additional GC pressure?
##########
modules/unsafe/src/main/java/org/apache/ignite/internal/util/GridUnsafe.java:
##########
@@ -1573,6 +1573,22 @@ public static long align(long size) {
return (size + (OBJ_ALIGN - 1L)) & (-OBJ_ALIGN);
}
+ /**
+ * @param src Buffer to copy from (length included).
+ * @param off Offset in source buffer.
+ * @param resBuf Result buffer.
+ * @param resOff Result offset.
+ * @param len Length.
Review Comment:
What do you think about this javadoc improvement?
`Length` -> `Number of bytes to copy from src to resBuf`
##########
modules/binary/api/src/main/java/org/apache/ignite/marshaller/MarshallerExclusions.java:
##########
Review Comment:
`Checks whether or not given class should be excluded from marshalling.` -
lets replace this comment with a better one:
`Checks whether given class should be excluded from marshalling.`
##########
modules/commons/src/main/java/org/apache/ignite/internal/util/CommonUtils.java:
##########
@@ -455,4 +491,253 @@ public static void writeUuid(DataOutput out, UUID uid)
throws IOException {
return null;
}
+
+ /**
+ * Get current Ignite name.
+ *
+ * @return Current Ignite name.
+ */
+ @Nullable public static String getCurrentIgniteName() {
+ return LOC_IGNITE_NAME.get();
+ }
+
+ /**
+ * Check if current Ignite name is set.
+ *
+ * @param name Name to check.
+ * @return {@code True} if set.
+ */
+ @SuppressWarnings("StringEquality")
+ public static boolean isCurrentIgniteNameSet(@Nullable String name) {
+ return name != LOC_IGNITE_NAME_EMPTY;
+ }
+
+ /**
+ * Set current Ignite name.
+ *
+ * @param newName New name.
+ * @return Old name.
+ */
+ @SuppressWarnings("StringEquality")
+ @Nullable public static String setCurrentIgniteName(@Nullable String
newName) {
+ String oldName = LOC_IGNITE_NAME.get();
+
+ if (oldName != newName)
+ LOC_IGNITE_NAME.set(newName);
+
+ return oldName;
+ }
+
+ /**
+ * Restore old Ignite name.
+ *
+ * @param oldName Old name.
+ * @param curName Current name.
+ */
+ @SuppressWarnings("StringEquality")
+ public static void restoreOldIgniteName(@Nullable String oldName,
@Nullable String curName) {
+ if (oldName != curName)
+ LOC_IGNITE_NAME.set(oldName);
+ }
+
+ /**
+ * @return Class loader used to load Ignite itself.
+ */
+ public static ClassLoader gridClassLoader() {
+ return gridClassLoader;
+ }
+
+ /**
+ * @param ldr Custom class loader.
+ * @param cfgLdr Class loader from config.
+ * @return ClassLoader passed as param in case it is not null or cfgLdr
in case it is not null or ClassLoader used to start Ignite.
+ */
+ public static ClassLoader resolveClassLoader(@Nullable ClassLoader ldr,
@Nullable ClassLoader cfgLdr) {
+ return (ldr != null && ldr != gridClassLoader)
+ ? ldr
+ : cfgLdr != null
+ ? cfgLdr
+ : gridClassLoader;
+ }
+
+ /**
+ * Tests whether or not given class is loadable provided class loader.
+ *
+ * @param clsName Class name to test.
+ * @param ldr Class loader to test with. If {@code null} - we'll use
system class loader instead.
+ * If System class loader is not set - this method will return {@code
false}.
+ * @return {@code True} if class is loadable, {@code false} otherwise.
+ */
+ public static boolean isLoadableBy(String clsName, @Nullable ClassLoader
ldr) {
+ assert clsName != null;
+
+ if (ldr == null)
+ ldr = gridClassLoader;
+
+ String lambdaParent = lambdaEnclosingClassName(clsName);
+
+ try {
+ ldr.loadClass(lambdaParent == null ? clsName : lambdaParent);
+
+ return true;
+ }
+ catch (ClassNotFoundException ignore) {
+ return false;
+ }
+ }
+
+ /**
+ * Gets class for the given name if it can be loaded or default given
class.
+ *
+ * @param cls Class.
+ * @param dflt Default class to return.
+ * @return Class or default given class if it can't be found.
+ */
+ @Nullable public static Class<?> classForName(@Nullable String cls,
@Nullable Class<?> dflt) {
+ return classForName(cls, dflt, false);
+ }
+
+ /**
+ * Gets class for the given name if it can be loaded or default given
class.
Review Comment:
`Gets a class for a given name if it can be loaded or a given default class
otherwise.` - the same is here.
##########
modules/binary/impl/src/main/java/org/apache/ignite/marshaller/jdk/JdkMarshallerImpl.java:
##########
@@ -75,14 +74,14 @@ public class JdkMarshaller extends
AbstractNodeNameAwareMarshaller {
* Use this constructor with caution. It creates a JdkMarshaller instance
that has class filtering DISABLED. Therefore,
* if it will be used on the server side to unmarshal user data received
from the network, it may lead to security breaches.
*/
- public JdkMarshaller() {
+ public JdkMarshallerImpl() {
this(null);
}
/**
* @param clsFilter Class name filter.
*/
- public JdkMarshaller(IgnitePredicate<String> clsFilter) {
+ public JdkMarshallerImpl(IgnitePredicate<String> clsFilter) {
Review Comment:
`Nullable` annotation on the parameter.
##########
modules/commons/src/main/java/org/apache/ignite/internal/util/CommonUtils.java:
##########
@@ -455,4 +491,253 @@ public static void writeUuid(DataOutput out, UUID uid)
throws IOException {
return null;
}
+
+ /**
+ * Get current Ignite name.
+ *
+ * @return Current Ignite name.
+ */
+ @Nullable public static String getCurrentIgniteName() {
+ return LOC_IGNITE_NAME.get();
+ }
+
+ /**
+ * Check if current Ignite name is set.
+ *
+ * @param name Name to check.
+ * @return {@code True} if set.
+ */
+ @SuppressWarnings("StringEquality")
+ public static boolean isCurrentIgniteNameSet(@Nullable String name) {
+ return name != LOC_IGNITE_NAME_EMPTY;
+ }
+
+ /**
+ * Set current Ignite name.
+ *
+ * @param newName New name.
+ * @return Old name.
+ */
+ @SuppressWarnings("StringEquality")
+ @Nullable public static String setCurrentIgniteName(@Nullable String
newName) {
+ String oldName = LOC_IGNITE_NAME.get();
+
+ if (oldName != newName)
+ LOC_IGNITE_NAME.set(newName);
+
+ return oldName;
+ }
+
+ /**
+ * Restore old Ignite name.
+ *
+ * @param oldName Old name.
+ * @param curName Current name.
+ */
+ @SuppressWarnings("StringEquality")
+ public static void restoreOldIgniteName(@Nullable String oldName,
@Nullable String curName) {
+ if (oldName != curName)
+ LOC_IGNITE_NAME.set(oldName);
+ }
+
+ /**
+ * @return Class loader used to load Ignite itself.
+ */
+ public static ClassLoader gridClassLoader() {
+ return gridClassLoader;
+ }
+
+ /**
+ * @param ldr Custom class loader.
+ * @param cfgLdr Class loader from config.
+ * @return ClassLoader passed as param in case it is not null or cfgLdr
in case it is not null or ClassLoader used to start Ignite.
+ */
+ public static ClassLoader resolveClassLoader(@Nullable ClassLoader ldr,
@Nullable ClassLoader cfgLdr) {
+ return (ldr != null && ldr != gridClassLoader)
+ ? ldr
+ : cfgLdr != null
+ ? cfgLdr
+ : gridClassLoader;
+ }
+
+ /**
+ * Tests whether or not given class is loadable provided class loader.
+ *
+ * @param clsName Class name to test.
+ * @param ldr Class loader to test with. If {@code null} - we'll use
system class loader instead.
+ * If System class loader is not set - this method will return {@code
false}.
+ * @return {@code True} if class is loadable, {@code false} otherwise.
+ */
+ public static boolean isLoadableBy(String clsName, @Nullable ClassLoader
ldr) {
+ assert clsName != null;
+
+ if (ldr == null)
+ ldr = gridClassLoader;
+
+ String lambdaParent = lambdaEnclosingClassName(clsName);
+
+ try {
+ ldr.loadClass(lambdaParent == null ? clsName : lambdaParent);
+
+ return true;
+ }
+ catch (ClassNotFoundException ignore) {
+ return false;
+ }
+ }
+
+ /**
+ * Gets class for the given name if it can be loaded or default given
class.
Review Comment:
`Gets a class for a given name if it can be loaded or a given default class
otherwise.` - this version is better.
##########
modules/binary/impl/src/main/java/org/apache/ignite/marshaller/jdk/JdkMarshallerObjectInputStream.java:
##########
Review Comment:
A null ref may be passed to the class constructor for `clsFilter` parameter,
it may make sense to add `Nullable` annotation.
##########
modules/commons/src/main/java/org/apache/ignite/internal/util/CommonUtils.java:
##########
@@ -455,4 +491,253 @@ public static void writeUuid(DataOutput out, UUID uid)
throws IOException {
return null;
}
+
+ /**
+ * Get current Ignite name.
+ *
+ * @return Current Ignite name.
+ */
+ @Nullable public static String getCurrentIgniteName() {
+ return LOC_IGNITE_NAME.get();
+ }
+
+ /**
+ * Check if current Ignite name is set.
+ *
+ * @param name Name to check.
+ * @return {@code True} if set.
+ */
+ @SuppressWarnings("StringEquality")
+ public static boolean isCurrentIgniteNameSet(@Nullable String name) {
+ return name != LOC_IGNITE_NAME_EMPTY;
+ }
+
+ /**
+ * Set current Ignite name.
+ *
+ * @param newName New name.
+ * @return Old name.
+ */
+ @SuppressWarnings("StringEquality")
+ @Nullable public static String setCurrentIgniteName(@Nullable String
newName) {
+ String oldName = LOC_IGNITE_NAME.get();
+
+ if (oldName != newName)
+ LOC_IGNITE_NAME.set(newName);
+
+ return oldName;
+ }
+
+ /**
+ * Restore old Ignite name.
+ *
+ * @param oldName Old name.
+ * @param curName Current name.
+ */
+ @SuppressWarnings("StringEquality")
+ public static void restoreOldIgniteName(@Nullable String oldName,
@Nullable String curName) {
+ if (oldName != curName)
+ LOC_IGNITE_NAME.set(oldName);
+ }
+
+ /**
+ * @return Class loader used to load Ignite itself.
+ */
+ public static ClassLoader gridClassLoader() {
+ return gridClassLoader;
+ }
+
+ /**
+ * @param ldr Custom class loader.
+ * @param cfgLdr Class loader from config.
+ * @return ClassLoader passed as param in case it is not null or cfgLdr
in case it is not null or ClassLoader used to start Ignite.
+ */
+ public static ClassLoader resolveClassLoader(@Nullable ClassLoader ldr,
@Nullable ClassLoader cfgLdr) {
+ return (ldr != null && ldr != gridClassLoader)
+ ? ldr
+ : cfgLdr != null
+ ? cfgLdr
+ : gridClassLoader;
+ }
+
+ /**
+ * Tests whether or not given class is loadable provided class loader.
+ *
+ * @param clsName Class name to test.
+ * @param ldr Class loader to test with. If {@code null} - we'll use
system class loader instead.
+ * If System class loader is not set - this method will return {@code
false}.
+ * @return {@code True} if class is loadable, {@code false} otherwise.
+ */
+ public static boolean isLoadableBy(String clsName, @Nullable ClassLoader
ldr) {
+ assert clsName != null;
+
+ if (ldr == null)
+ ldr = gridClassLoader;
+
+ String lambdaParent = lambdaEnclosingClassName(clsName);
+
+ try {
+ ldr.loadClass(lambdaParent == null ? clsName : lambdaParent);
+
+ return true;
+ }
+ catch (ClassNotFoundException ignore) {
+ return false;
+ }
+ }
+
+ /**
+ * Gets class for the given name if it can be loaded or default given
class.
+ *
+ * @param cls Class.
+ * @param dflt Default class to return.
+ * @return Class or default given class if it can't be found.
+ */
+ @Nullable public static Class<?> classForName(@Nullable String cls,
@Nullable Class<?> dflt) {
+ return classForName(cls, dflt, false);
+ }
+
+ /**
+ * Gets class for the given name if it can be loaded or default given
class.
+ *
+ * @param cls Class.
+ * @param dflt Default class to return.
+ * @param includePrimitiveTypes Whether class resolution should include
primitive types
+ * (i.e. "int" will resolve to int.class if
flag is set)
+ * @return Class or default given class if it can't be found.
+ */
+ @Nullable public static Class<?> classForName(
+ @Nullable String cls,
+ @Nullable Class<?> dflt,
+ boolean includePrimitiveTypes
+ ) {
+ Class<?> clazz;
+ if (cls == null)
+ clazz = dflt;
+ else if (!includePrimitiveTypes || cls.length() > 7 || (clazz =
primitiveMap.get(cls)) == null) {
+ try {
+ clazz = Class.forName(cls);
+ }
+ catch (ClassNotFoundException ignore) {
+ clazz = dflt;
+ }
+ }
+ return clazz;
+ }
+
+ /**
+ * Gets class for provided name. Accepts primitive types names.
+ *
+ * @param clsName Class name.
+ * @param ldr Class loader.
+ * @param useCache If true class loader and result should be cached
internally, false otherwise.
+ * @return Class.
+ * @throws ClassNotFoundException If class not found.
+ */
+ public static Class<?> forName(
+ String clsName,
+ @Nullable ClassLoader ldr,
+ IgnitePredicate<String> clsFilter,
Review Comment:
`clsFilter` could be null, please add `Nullable` annotation on it here.
##########
modules/unsafe/src/main/java/org/apache/ignite/internal/util/GridUnsafe.java:
##########
@@ -1573,6 +1573,22 @@ public static long align(long size) {
return (size + (OBJ_ALIGN - 1L)) & (-OBJ_ALIGN);
}
+ /**
+ * @param src Buffer to copy from (length included).
+ * @param off Offset in source buffer.
+ * @param resBuf Result buffer.
+ * @param resOff Result offset.
+ * @param len Length.
+ * @return Number of bytes overwritten in {@code bytes} array.
Review Comment:
I believe this description is wrong. Method returns a maximum index in
resBuf affected by the copy operation. But the number of bytes overwritten is
just `len`.
BTW I wonder if we could drop the return value at all. It is never used
across all callers of this method and doesn't make a lot of sence to me. It can
be calculated on a caller side easily and doesn't depend on actual method
execution.
##########
modules/binary/api/src/main/java/org/apache/ignite/marshaller/MarshallersFactory.java:
##########
Review Comment:
How about renaming this class to `JdkMarshallersFactory` and `Marshallers`
class to `MarshallersFactory`?
Thus we would make these classes closer to their roles: right now
`Marshallers` class is basically a factory but we cannot name it a
`MarshallersFactory` as this name is already taken.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
