You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/04/16 11:55:19 UTC

[GitHub] [ignite] ivandasch opened a new pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.

ivandasch opened a new pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.
URL: https://github.com/apache/ignite/pull/7679
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.
URL: https://github.com/apache/ignite/pull/7679#discussion_r409596733
 
 

 ##########
 File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java
 ##########
 @@ -5717,4 +5811,72 @@ private Object readResolve() {
             }
         }
     }
+
+    /** */
+    static class TestPlatformMemory implements PlatformMemory {
+        /** */
+        private static final int DEFAULT_CAPACITY = 1024;
+
+        /** */
+        long memPtr;
 
 Review comment:
   Can be `private`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.
URL: https://github.com/apache/ignite/pull/7679#discussion_r409601720
 
 

 ##########
 File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java
 ##########
 @@ -2943,6 +2953,90 @@ public void testClassFieldsMarshalling() throws Exception {
         assertEquals(obj.cls1, cls1);
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testReadDetachedMap_WithoutFullyCopyingUnderlyingData() throws Exception {
+        Map<Key, Value> map = IntStream.range(0, 1000).mapToObj(i -> new T2<>(new Key(i), new Value(i)))
+            .collect(Collectors.toMap(T2::getKey, T2::getValue));
+
+        Map<BinaryObject, BinaryObject> desMap =
+            (Map<BinaryObject, BinaryObject>)ensureAllDataNotCopiedOnHeap_WhenObjectReadDetached(map);
+
+        desMap.forEach((k, v) -> {
+            Key key = new Key(k.field("key"));
+            Value val = new Value(v.field("val"));
+
+            assertTrue(map.containsKey(key));
+            assertEquals(val, map.get(key));
+        });
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testReadDetachedCollection_WithoutFullyCopyingUnderlyingData() throws Exception {
+        Collection<Value> col = IntStream.range(0, 1000).mapToObj(Value::new).collect(Collectors.toSet());
+
+        Collection<BinaryObject> desCol =
+            (Collection<BinaryObject>)ensureAllDataNotCopiedOnHeap_WhenObjectReadDetached(col);
+
+        desCol.forEach(v -> {
 
 Review comment:
   Should size be checked too?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.
URL: https://github.com/apache/ignite/pull/7679#discussion_r409605269
 
 

 ##########
 File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java
 ##########
 @@ -2943,6 +2953,90 @@ public void testClassFieldsMarshalling() throws Exception {
         assertEquals(obj.cls1, cls1);
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testReadDetachedMap_WithoutFullyCopyingUnderlyingData() throws Exception {
+        Map<Key, Value> map = IntStream.range(0, 1000).mapToObj(i -> new T2<>(new Key(i), new Value(i)))
+            .collect(Collectors.toMap(T2::getKey, T2::getValue));
+
+        Map<BinaryObject, BinaryObject> desMap =
+            (Map<BinaryObject, BinaryObject>)ensureAllDataNotCopiedOnHeap_WhenObjectReadDetached(map);
+
+        desMap.forEach((k, v) -> {
+            Key key = new Key(k.field("key"));
+            Value val = new Value(v.field("val"));
+
+            assertTrue(map.containsKey(key));
+            assertEquals(val, map.get(key));
+        });
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testReadDetachedCollection_WithoutFullyCopyingUnderlyingData() throws Exception {
+        Collection<Value> col = IntStream.range(0, 1000).mapToObj(Value::new).collect(Collectors.toSet());
+
+        Collection<BinaryObject> desCol =
+            (Collection<BinaryObject>)ensureAllDataNotCopiedOnHeap_WhenObjectReadDetached(col);
+
+        desCol.forEach(v -> {
+           Value val = new Value(v.field("val"));
+
+           assertTrue(col.contains(val));
+        });
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testReadDetachedArray_WithoutFullyCopyingUnderlyingData() throws Exception {
+        Value[] arr = IntStream.range(0, 1000).mapToObj(Value::new).toArray(Value[]::new);
+
+        Object[] desArr = (Object[])ensureAllDataNotCopiedOnHeap_WhenObjectReadDetached(arr);
+
+        assertEquals(arr.length, desArr.length);
+
+        for (int i = 0; i < arr.length; i++) {
+            BinaryObject val = (BinaryObject)desArr[i];
+
+            assertEquals(arr[i], new Value(val.field("val")));
+        }
+    }
+
+    /**
+     * Tests that process of detach reading object from platform offheap memory
+     * performs correctly and doesn't invoke additional memory copy on heap.
+     *
+     * @param testObj Test object to perform marshalling and detached unmarshalling.
+     * @return Unmarshalled object.
+     * @throws Exception If failed.
+     */
+    private Object ensureAllDataNotCopiedOnHeap_WhenObjectReadDetached(Object testObj) throws Exception {
 
 Review comment:
   I suggest simplify name to `ensureAllDataNotCopiedOnHeap`. I think javadoc is informative.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.
URL: https://github.com/apache/ignite/pull/7679#discussion_r409594364
 
 

 ##########
 File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java
 ##########
 @@ -5717,4 +5811,72 @@ private Object readResolve() {
             }
         }
     }
+
+    /** */
+    static class TestPlatformMemory implements PlatformMemory {
+        /** */
+        private static final int DEFAULT_CAPACITY = 1024;
+
+        /** */
+        long memPtr;
+
+        /** */
+        int capacity;
+
+        /** */
+        public TestPlatformMemory() {
+            this(DEFAULT_CAPACITY);
+        }
+
+        /** */
 
 Review comment:
   Add javadoc, please

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7679: IGNITE-12827 Fix skipped detach flag when unmarshalling collections and object arrays.
URL: https://github.com/apache/ignite/pull/7679#discussion_r409596787
 
 

 ##########
 File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java
 ##########
 @@ -5717,4 +5811,72 @@ private Object readResolve() {
             }
         }
     }
+
+    /** */
+    static class TestPlatformMemory implements PlatformMemory {
+        /** */
+        private static final int DEFAULT_CAPACITY = 1024;
+
+        /** */
+        long memPtr;
+
+        /** */
+        int capacity;
 
 Review comment:
   Can be `private`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services