johnzon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Struberg <strub...@yahoo.de.INVALID>
Subject Re: johnzon git commit: adding a broken test to show why previous commit broke the buffer strategies
Date Tue, 26 Sep 2017 08:16:57 GMT
I've reviewed the cache together with Reinhard in a hangout pair programming session on Friday
and it looks good imo.

LieGrue,
strub


> Am 26.09.2017 um 09:56 schrieb Romain Manni-Bucau <rmannibucau@gmail.com>:
> 
> then the only blocker is to guarantee we will never add back to any cache
> the extended buffer. This can just be a matter of reviewing our strategies
> but didn't get a chance to do it yet. Can surely have a look this week-end
> but fear i'll not be able before :(
> 
> 
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau>
|
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
> 
> 2017-09-26 9:22 GMT+02:00 Mark Struberg <struberg@yahoo.de.invalid>:
> 
>> Hmm maybe, but in a later version, ok?
>> 
>> Would love to start the release today, ok?
>> 
>> LieGrue,
>> Strub
>> 
>>> Am 26.09.2017 um 08:10 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
>>> :
>>> 
>>> We already have it, what is important with this auto extend logic is a
>>> toggle flag to add back in the cache either the original ("small") buffer
>>> or the extended one ("big"). We dont have that support yet. But in all
>>> cases we should call release to let the strategy have a chance to cleanup
>>> things.
>>> 
>>> Le 26 sept. 2017 08:01, "Mark Struberg" <struberg@yahoo.de.invalid> a
>>> écrit :
>>> 
>>>> Hmm, what about a keepMaxBuffers = n config?
>>>> 
>>>> By default it would be e.g. 50.
>>>> Means if the queue is > n we do not return it.
>>>> By configure 0 as n we automatically have what you want, right?
>>>> 
>>>> LieGrue,
>>>> strub
>>>> 
>>>> 
>>>>> Am 26.09.2017 um 06:59 schrieb Romain Manni-Bucau <
>> rmannibucau@gmail.com
>>>>> :
>>>>> 
>>>>> We should get a config about it and ensure we release() *any* buffer
in
>>>>> case we go native (validate our lifecycle). This mean we should
>>>> distinguish
>>>>> between release (end of buffer usage) and going back to pool (or the
>>>>> opposite: newPoolableBuffer vs newVolatileBuffer).
>>>>> 
>>>>> The test is not super good yet but a start, it misses some size
>> asserts.
>>>>> 
>>>>> 
>>>>> Le 25 sept. 2017 23:44, "Mark Struberg" <struberg@yahoo.de.invalid>
a
>>>>> écrit :
>>>>> 
>>>>>> Not quite sure what this should tell us.
>>>>>> 
>>>>>> Of course the exceeded buffer doesn't get returned!
>>>>>> We might probably return the original one and thus improve our logic.
>>>>>> But it's not strictly wrong the way it currently behaves.
>>>>>> It's now way faster under normal conditions and it consumes way less
>> mem
>>>>>> than before.
>>>>>> 
>>>>>> LieGrue,
>>>>>> strub
>>>>>> 
>>>>>>> Am 25.09.2017 um 08:22 schrieb rmannibucau@apache.org:
>>>>>>> 
>>>>>>> Repository: johnzon
>>>>>>> Updated Branches:
>>>>>>> refs/heads/master cc24e8e1c -> 60f48cf65
>>>>>>> 
>>>>>>> 
>>>>>>> adding a broken test to show why previous commit broke the buffer
>>>>>> strategies
>>>>>>> 
>>>>>>> 
>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/johnzon/repo
>>>>>>> Commit: http://git-wip-us.apache.org/repos/asf/johnzon/commit/
>> 60f48cf6
>>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/johnzon/tree/60f48cf6
>>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/johnzon/diff/60f48cf6
>>>>>>> 
>>>>>>> Branch: refs/heads/master
>>>>>>> Commit: 60f48cf65708e3a9bc9943b6d5cac0435772b6e9
>>>>>>> Parents: cc24e8e
>>>>>>> Author: Romain Manni-Bucau <rmannibucau@gmail.com>
>>>>>>> Authored: Mon Sep 25 08:22:26 2017 +0200
>>>>>>> Committer: Romain Manni-Bucau <rmannibucau@gmail.com>
>>>>>>> Committed: Mon Sep 25 08:22:26 2017 +0200
>>>>>>> 
>>>>>>> ------------------------------------------------------------
>> ----------
>>>>>>> .../apache/johnzon/core/BrokenDefaultTest.java  | 101
>>>>>> +++++++++++++++++++
>>>>>>> 1 file changed, 101 insertions(+)
>>>>>>> ------------------------------------------------------------
>> ----------
>>>>>>> 
>>>>>>> 
>>>>>>> http://git-wip-us.apache.org/repos/asf/johnzon/blob/
>>>>>> 60f48cf6/johnzon-core/src/test/java/org/apache/johnzon/
>>>>>> core/BrokenDefaultTest.java
>>>>>>> ------------------------------------------------------------
>> ----------
>>>>>>> diff --git a/johnzon-core/src/test/java/org/apache/johnzon/core/
>>>> BrokenDefaultTest.java
>>>>>> b/johnzon-core/src/test/java/org/apache/johnzon/core/
>>>>>> BrokenDefaultTest.java
>>>>>>> new file mode 100644
>>>>>>> index 0000000..ec63d9f
>>>>>>> --- /dev/null
>>>>>>> +++ b/johnzon-core/src/test/java/org/apache/johnzon/core/
>>>>>> BrokenDefaultTest.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.johnzon.core;
>>>>>>> +
>>>>>>> +import static java.util.Collections.emptyMap;
>>>>>>> +import static org.junit.Assert.assertEquals;
>>>>>>> +
>>>>>>> +import java.io.ByteArrayInputStream;
>>>>>>> +import java.io.IOException;
>>>>>>> +import java.io.InputStream;
>>>>>>> +import java.lang.reflect.Field;
>>>>>>> +import java.nio.charset.StandardCharsets;
>>>>>>> +import java.util.Queue;
>>>>>>> +
>>>>>>> +import javax.json.Json;
>>>>>>> +import javax.json.stream.JsonParser;
>>>>>>> +import javax.json.stream.JsonParserFactory;
>>>>>>> +
>>>>>>> +import org.junit.Ignore;
>>>>>>> +import org.junit.Test;
>>>>>>> +
>>>>>>> +public class BrokenDefaultTest {
>>>>>>> +
>>>>>>> +    @Test
>>>>>>> +    @Ignore("buggy but pushing to share the use case")
>>>>>>> +    public void run() throws NoSuchFieldException,
>>>>>> IllegalAccessException { // shouldnt fail by default
>>>>>>> +        final JsonParserFactory factory = Json.createParserFactory(
>>>>>> emptyMap());
>>>>>>> +        final int length = 1024 * 1024;
>>>>>>> +        assertEquals(0, get(Queue.class, get(
>>>>>>> +                BufferStrategy.BufferProvider.class, factory,
>>>>>> "bufferProvider"), "queue").size());
>>>>>>> +        try (final JsonParser parser = factory.createParser(
>>>> newDynamicInput(length)))
>>>>>> {
>>>>>>> +            int eventCount = 0;
>>>>>>> +            while (parser.hasNext()) {
>>>>>>> +                eventCount++;
>>>>>>> +                final JsonParser.Event next = parser.next();
>>>>>>> +                if (eventCount == 2 && next ==
>>>>>> JsonParser.Event.VALUE_STRING) {
>>>>>>> +                    assertEquals(length,
>> parser.getString().length());
>>>>>>> +                }
>>>>>>> +            }
>>>>>>> +        }
>>>>>>> +        assertEquals(1, get(Queue.class, get(
>>>>>>> +                BufferStrategy.BufferProvider.class, factory,
>>>>>> "bufferProvider"), "queue").size());
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    private <T> T get(final Class<T> returnType,
final Object
>>>> instance,
>>>>>> final String field)
>>>>>>> +            throws NoSuchFieldException, IllegalAccessException
{
>>>>>>> +        Class<?> current = instance.getClass();
>>>>>>> +        while (current != Object.class) {
>>>>>>> +            try {
>>>>>>> +                final Field declaredField =
>> current.getDeclaredField(
>>>>>> field);
>>>>>>> +                if (!declaredField.isAccessible()) {
>>>>>>> +                    declaredField.setAccessible(true);
>>>>>>> +                }
>>>>>>> +                return returnType.cast(declaredField.
>> get(instance));
>>>>>>> +            } catch (final NoSuchFieldException nsfe) {
>>>>>>> +                current = current.getSuperclass();
>>>>>>> +            }
>>>>>>> +        }
>>>>>>> +        throw new IllegalAccessError(instance + " field: " +
field);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    private InputStream newDynamicInput(final int size) {
>>>>>>> +        return new InputStream() {
>>>>>>> +
>>>>>>> +            private InputStream before = new
>>>>>> ByteArrayInputStream("{\"key\":\"".getBytes(StandardCharsets.UTF_8));
>>>>>>> +
>>>>>>> +            private InputStream after = new
>>>> ByteArrayInputStream("\"}".
>>>>>> getBytes(StandardCharsets.UTF_8));
>>>>>>> +
>>>>>>> +            private int remaining = size;
>>>>>>> +
>>>>>>> +            @Override
>>>>>>> +            public int read() throws IOException {
>>>>>>> +                {
>>>>>>> +                    final int val = before.read();
>>>>>>> +                    if (val >= 0) {
>>>>>>> +                        return val;
>>>>>>> +                    }
>>>>>>> +                }
>>>>>>> +                if (remaining < 0) {
>>>>>>> +                    return after.read();
>>>>>>> +                }
>>>>>>> +                remaining--;
>>>>>>> +                return 'a';
>>>>>>> +            }
>>>>>>> +        };
>>>>>>> +    }
>>>>>>> +}
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> .
>>>>>> 
>>>> 
>>>> 
>> 
>> 


Mime
View raw message