Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection querying in Portables #8132

Merged
merged 11 commits into from May 13, 2016

Conversation

Projects
None yet
4 participants
@tombujok
Copy link
Contributor

commented May 9, 2016

No description provided.

@tombujok tombujok added this to the 3.6.3 milestone May 9, 2016

@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2016

If you somebody is not familiar with this feature here's some context info:
http://docs.hazelcast.org/docs/3.6/manual/html-single/index.html#querying-in-collections-and-arrays
http://docs.hazelcast.org/docs/3.6/manual/html-single/index.html#custom-attributes
http://docs.hazelcast.org/docs/3.6/manual/html-single/index.html#implementing-portable-serialization

Here are the samples related to this piece of code:
https://github.com/hazelcast/hazelcast-code-samples/tree/master/distributed-map/custom-attributes
https://github.com/hazelcast/hazelcast-code-samples/tree/master/distributed-map/query-collections

This feature:

  • allows querying a collection like car.wheels[0].pressure or car.wheels[any].pressure in the Portable data format
  • allows using a ValueExtractor in the Portable data format

Earlier we implemented this feature for plain POJOs without portable support. This is the extension of this feature for portables. Sidenote: in the Portable data format there's no collections allowed - just java arrays.

Second commit fixes #8134

@tombujok tombujok force-pushed the tombujok:portables-pr branch from 247b5f6 to 42e4dbb May 9, 2016

@tombujok tombujok force-pushed the tombujok:portables-pr branch from 42e4dbb to be53954 May 9, 2016

@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2016

run-lab-run

@jerrinot

This comment has been minimized.

Copy link
Contributor

commented May 10, 2016

contract of ValueExtractor should describe how to use it together with Portable serialization

Update by tombujok: Fixed.

* limitations under the License.
*/

package com.hazelcast.query.extractor;

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

does this belong to this package?

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

Yes, it is related to extraction with extractors only

int getClassId();

/**
* Determines type of position. There's a PortableSinglePosition and PortableSinglePosition (although both private)

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 10, 2016

Contributor

just a typo here (I guess should be PortableSinglePosition and PortableMultiPosition?)

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

thx! comment added

* @return the type of the field under the leaf of the given path
*/
@Nullable
FieldType getType();

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 10, 2016

Contributor

What does a null return value mean? This position is not a leaf? Or some kind of error?

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

thx! comment added

@@ -156,15 +156,23 @@ private Object getParentObject(Object obj) throws Exception {

private void reduceArrayInto(MultiResult collector, Object[] currentObject) {
Object[] array = currentObject;
for (int i = 0; i < array.length; i++) {
collector.add(array[i]);
if (array.length == 0) {

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

can you describe a rationale behind these changes? (this and the method with collections)

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

it's for this test case

    @Test
    public void comparable_primitive_reduced_atLeaf_comparedToNull_matching() {
        execute(Input.of(BOND, KRUEGER),
                Query.of(equal("limbs_[any].tattoos_[any]", null), mv),
                Expected.of(BOND, KRUEGER));
    }

Both input object have empty tattoos array/list.
they query would work fine if you changed any to a concrete number. With any it didn't return a match with null and it should

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

Without that fix the result was empty which was incorrect.

// This part will be improved in 3.7 to avoid extra allocation
DefaultValueCollector collector = new DefaultValueCollector();
extractor.extract(target, arguments, collector);
if (target instanceof Data) {

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

is this check enough to conclude it's Portable?
Data has a method isPortable()

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor
@@ -92,4 +92,16 @@ static String extractAttributeNameNameWithoutArguments(String attributeNameWithA
}
}

public static int indexOf(char[] input, char splitter, int offset) {

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

StringUtil ?

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

we don't have commons-lang on the classpath and I want to control the implementation to be sure it's fast for the case.

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 11, 2016

Contributor

we have our own StringUtil.

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

Done

/**
* Method for reusable generic getters
*/
Object getValue(Object obj, String attributePath) throws Exception {

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

I'm not sure what Method for reusable generic getters means

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

thx! comment added

@@ -24,6 +24,8 @@
/**
* Provides a mean of reading portable fields from a binary in form of java primitives
* arrays of java primitives , nested portable fields and array of portable fields.
* <p/>
* fieldName used in most of the methods may contain nested fields, like body.brain.iq

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

a nitpicking: it's a bit too informal language, for my taste anyway.

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

Sure. Earlier it was not clear to me that it may be a nested path. Could you rephrase it to a more formal sentence?

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 10, 2016

Contributor

what about this: PortableReader support nested paths. For example <code>body.brain.iq</code> is a valid field name.

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

thx! comment changed

@@ -55,4 +71,32 @@ public static FieldType get(byte type) {
return ALL[type];
}

public boolean isArrayType() {
return type >= PORTABLE_ARRAY.type;

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 10, 2016

Contributor

This is fragile, why not add a separate boolean property in enum values?

This comment has been minimized.

Copy link
@tombujok

tombujok May 10, 2016

Author Contributor

Sure, I'm fully aware of it and I was expecting these comments :)
It's kind of a common-sense solution since there's no more types in the portable supported.
So it's not like this that there will be new types added.
I also added an explicit test case for it, so if somebody changes sth here the test will break too.
The advantage of the isArrayType and getSingleType is that they are blazing fast.
One is just an int comparison, the other one is a modulo. This code is invoked for each MapEntry so I was trying to keep it fast.
I disagree that it's easy to break it in this case (tests + no more types in portable) but I'm open to change if sb proposes a solution that's that fast and doesn't require an allocation of a Map.

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 10, 2016

Contributor

I was actually thinking of something like this, explicitly setting up the properties that describe the FieldTypes and returning them from isArrayType and getSingleType. Apart from being at least as fast, I think it also models more explicitly what FieldType is about (where being explicit can be considered either good, or bad :) ).

public enum FieldType {
    // SINGLE-VALUE TYPES
    PORTABLE(0, MAX_VALUE, 0, false),
    BYTE(1, BYTE_SIZE_IN_BYTES, 1, false),
....
    // ARRAY TYPES
    PORTABLE_ARRAY(10, MAX_VALUE, 0, true),
    BYTE_ARRAY(11, MAX_VALUE, 1, true),
.....
     private static final FieldType[] ALL = FieldType.values();

    private final byte type;
    private final byte elementType;
    private final int elementSize;
    private final boolean arrayType;

    FieldType(int type, int elementSize, int elementType, boolean arrayType) {
        this.type = (byte) type;
        this.elementSize = elementSize;
        this.elementType = (byte) elementType;
        this.arrayType = arrayType;
    }

    public byte getId() {
        return type;
    }

    public static FieldType get(byte type) {
        return ALL[type];
    }

    public boolean isArrayType() {
        return arrayType;
    }

    public FieldType getSingleType() {
        if (arrayType) {
            return get(elementType);
        }
        return this;
    }

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

Thx Vassilis.I appreciate your input here - I guess it would have been a bit cleaner, but I don't really see a big improvement here. If you & others agree I would prefer to skip that fix.

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 12, 2016

Contributor

Agreed, the test will catch undesired modifications. Thanks!

public FieldType getSingleType() {
if (isArrayType()) {
// GOTCHA: Wont' work if you add more types!!!
return get((byte) (getId() % TYPES_COUNT));

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 10, 2016

Contributor

Also easy to break, as far as I understand FieldType boils down to (typeId, elementSize, multiplicity), so I would favor explicitly separating the fieldTypeId (e.g. 10 for PORTABLE_ARRAY) from the element typeId (which would be 0 for both PORTABLE and PORTABLE_ARRAY). Wdyt?

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

Plz see above.

/**
* Can't be accessed concurrently
*/
public class DefaultPortableReader extends ValueReader implements PortableReader {

This comment has been minimized.

Copy link
@pveentjer

pveentjer May 11, 2016

Member

If you need to query n objects, how many of these DefaultPortableReader instances are created?

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

It's as bad as it was before. One reader per entry.
https://github.com/tombujok/hazelcast/blob/106e89f207ef891e0b4b7fae736abf91e171a3cd/hazelcast/src/main/java/com/hazelcast/query/impl/PortableExtractor.java#L66-L66

I'm planning to turn the whole thing upside and optimise not only that but a also couple of other sub-optimal things in 3.8, but for this PR it was out of scope.

@pveentjer

This comment has been minimized.

Copy link
Member

commented May 11, 2016

Are there benchmark that verify:
1:the performance impact on queries using portables that do not make use of this feature. We need to make sure there is no regression.
2: the performance of queries that do make use of this feature.

Update by tombujok: answered below.

@@ -180,12 +181,18 @@ public ClassDefinition lookupOrRegisterClassDefinition(Portable p) throws IOExce
public FieldDefinition getFieldDefinition(ClassDefinition classDef, String name) {

This comment has been minimized.

Copy link
@pveentjer

pveentjer May 11, 2016

Member

How often per query is this executed? Is this cached?

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

The same as before. Once per entry.
TODO added.

*
* @param <T> type of the extracted value
*/
public abstract class ValueCallback<T> {

This comment has been minimized.

Copy link
@pveentjer

pveentjer May 11, 2016

Member

Is this public API? I guess so since it isn't in an impl/internal package.

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

Yes, it is.

public final class Extractors {

private static final int MAX_CLASSES_IN_CACHE = 1000;
private static final int MAX_GETTERS_PER_CLASS_IN_CACHE = 100;
private static final float EVICTION_PERCENTAGE = 0.2f;
private static final Extractors EMPTY = new Extractors(Collections.<MapAttributeConfig>emptyList());

private volatile PortableGetter genericPortableGetter;

This comment has been minimized.

Copy link
@pveentjer

pveentjer May 11, 2016

Member

Doesn't it make sense to construct this class with a SerializationService as field instead of needing to pass it in the extra method?

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

Yes, probably it would be good too. I'll keep that in mind for future changes if you agree.
it's more like a style-related thing and it will require changes in the enterprise and in all places that use the Extractors.empty()

This comment has been minimized.

Copy link
@pveentjer

pveentjer May 11, 2016

Member

if you add a todo; I'm fine with it.

This comment has been minimized.

Copy link
@tombujok

tombujok May 12, 2016

Author Contributor

Done.

return serializationService.toObject(target);
}

// at this stage if it's Data then it's a Portable

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 11, 2016

Contributor

is this comment right?

let's assume I assume I use Portable serialization. there are 2 cases

  • IN-MEMORY-FORMAT is set to OBJECT - then it exit this method at line 81
  • IN-MEMORY-FORMAT is set to BINARY/NATIVE - then it exit this method at line 88

-> If you are at this stage then Portable serialization is not used at all.

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

This comment is misleading indeed. In the case of portable it will exit in lines you pointed out.

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

Fixed

}

// convert non-portable Data to object
if (target instanceof Data) {

This comment has been minimized.

Copy link
@jerrinot

jerrinot May 11, 2016

Contributor

this check might be redundant as serialization service is doing the same thing. however it adds to readability?

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

Yeah, exactly. it kind of enables understanding the flow of ifs (at least to me). If it doesn't bother you too much I would prefer to keep it like this.

@jerrinot

This comment has been minimized.

Copy link
Contributor

commented May 11, 2016

I like the implementation a lot.
I have love-hate relationship with some test classes (generated tests..). However this is best to be addressed by adding comments explaining the idea behind "generated tests". I appreciate thoroughness & consistency of the tests.

I also really like the fact you caught & addressed the performance regression up-front!

if (target instanceof Data) {
return serializationService.toObject(target);
}

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris May 11, 2016

Contributor

Minor one, perhaps integrate this if-block into previous one so it becomes:

        if (target instanceof Data) {
            targetData = (Data) target;
            if (targetData.isPortable()) {
                return targetData;
            }
            else {
                // convert non-portable Data to object
                return serializationService.toObject(target);
            }
        }

This comment has been minimized.

Copy link
@tombujok

tombujok May 11, 2016

Author Contributor

Good catch. Fixed!

@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 11, 2016

@jerrinot Thx Jaromir. I don't like the complexity of the parametrised DefaultPortableReaderSpecTest test either - I was fine with deleting it before (even now). The problem is that the DefaultPortableReader has tens of methods and there's a couple of tricky nested read combinations that used to cause a lot of problems.
As said, I tried to have manual tests only for this class, but it was impossible to cover the tricky cases. On the upside, this test is the backward-compatibility guard for the reader from now on (as there was very poor test coverage there before). While working on refactoring it saved me a couple of times in very unexpected places...

@pveentjer Yes, there is a JMH performance test in this PR. I'll post the results while my tuning is done.
https://github.com/tombujok/hazelcast/blob/127e4be17510c4582c4a5c78e248bf53911f9610/hazelcast/src/test/java/com/hazelcast/nio/serialization/impl/DefaultPortableReaderPerformanceTest.java#L59-L59

tombujok added some commits May 11, 2016

@tombujok tombujok force-pushed the tombujok:portables-pr branch from 3a0e325 to 8af891f May 12, 2016

@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2016

Results of the micro-benchmarks of the DefaultPortableReader after the last changes

master:
-------------

Benchmark                                                                 Mode   Cnt     Score    Error  Units
DefaultPortableReaderPerformanceTest.readBoolean                          avgt  2000    26.015 ±  0.273  ns/op
DefaultPortableReaderPerformanceTest.readBooleanArray                     avgt  2000    33.415 ±  0.334  ns/op
DefaultPortableReaderPerformanceTest.readByte                             avgt  2000    24.460 ±  0.241  ns/op
DefaultPortableReaderPerformanceTest.readByteArray                        avgt  2000    30.172 ±  0.538  ns/op
DefaultPortableReaderPerformanceTest.readChar                             avgt  2000    28.510 ±  0.276  ns/op
DefaultPortableReaderPerformanceTest.readCharArray                        avgt  2000    30.166 ±  0.279  ns/op
DefaultPortableReaderPerformanceTest.readDouble                           avgt  2000    33.084 ±  0.325  ns/op
DefaultPortableReaderPerformanceTest.readDoubleArray                      avgt  2000    33.593 ±  0.340  ns/op
DefaultPortableReaderPerformanceTest.readFloat                            avgt  2000    29.960 ±  0.296  ns/op
DefaultPortableReaderPerformanceTest.readFloatArray                       avgt  2000    30.963 ±  0.302  ns/op
DefaultPortableReaderPerformanceTest.readInt                              avgt  2000    29.384 ±  0.283  ns/op
DefaultPortableReaderPerformanceTest.readIntArray                         avgt  2000    30.669 ±  0.302  ns/op
DefaultPortableReaderPerformanceTest.readLong                             avgt  2000    32.256 ±  0.306  ns/op
DefaultPortableReaderPerformanceTest.readLongArray                        avgt  2000    30.881 ±  0.285  ns/op
DefaultPortableReaderPerformanceTest.readShort                            avgt  2000    28.593 ±  0.283  ns/op
DefaultPortableReaderPerformanceTest.readShortArray                       avgt  2000    31.540 ±  0.318  ns/op
DefaultPortableReaderPerformanceTest.readUTF                              avgt  2000    30.389 ±  0.286  ns/op
DefaultPortableReaderPerformanceTest.readUTFArray                         avgt  2000    31.556 ±  0.308  ns/op

DefaultPortableReaderPerformanceTest.readPortable                         avgt  2000   395.670 ±  4.526  ns/op
DefaultPortableReaderPerformanceTest.readPortableArray                    avgt  2000  1955.495 ± 20.277  ns/op

DefaultPortableReaderPerformanceTest.readPortableInt_nested               avgt  2000   358.275 ± 14.170  ns/op
DefaultPortableReaderPerformanceTest.readPortablePortableInt_nestedTwice  avgt  2000   504.691 ±  6.901  ns/op




This PR :
--------------

Benchmark                                                                 Mode   Cnt     Score    Error  Units
DefaultPortableReaderPerformanceTest.readBoolean                          avgt  2000    43.211 ±  0.448  ns/op
DefaultPortableReaderPerformanceTest.readBooleanArray                     avgt  2000    56.259 ±  0.564  ns/op
DefaultPortableReaderPerformanceTest.readByte                             avgt  2000    41.592 ±  0.419  ns/op
DefaultPortableReaderPerformanceTest.readByteArray                        avgt  2000    41.778 ±  0.425  ns/op
DefaultPortableReaderPerformanceTest.readChar                             avgt  2000    45.808 ±  0.453  ns/op
DefaultPortableReaderPerformanceTest.readCharArray                        avgt  2000    57.816 ±  0.626  ns/op
DefaultPortableReaderPerformanceTest.readDouble                           avgt  2000    46.454 ±  0.521  ns/op
DefaultPortableReaderPerformanceTest.readDoubleArray                      avgt  2000    63.882 ±  0.645  ns/op
DefaultPortableReaderPerformanceTest.readFloat                            avgt  2000    44.396 ±  0.464  ns/op
DefaultPortableReaderPerformanceTest.readFloatArray                       avgt  2000    53.926 ±  0.572  ns/op
DefaultPortableReaderPerformanceTest.readInt                              avgt  2000    44.534 ±  0.451  ns/op
DefaultPortableReaderPerformanceTest.readIntArray                         avgt  2000    54.809 ±  0.541  ns/op
DefaultPortableReaderPerformanceTest.readLong                             avgt  2000    62.276 ±  0.620  ns/op
DefaultPortableReaderPerformanceTest.readLongArray                        avgt  2000    54.799 ±  0.590  ns/op
DefaultPortableReaderPerformanceTest.readShort                            avgt  2000    40.176 ±  0.392  ns/op
DefaultPortableReaderPerformanceTest.readShortArray                       avgt  2000    53.906 ±  0.548  ns/op
DefaultPortableReaderPerformanceTest.readUTF                              avgt  2000    42.896 ±  0.433  ns/op
DefaultPortableReaderPerformanceTest.readUTFArray                         avgt  2000    53.434 ±  0.522  ns/op

DefaultPortableReaderPerformanceTest.readPortable                         avgt  2000   576.204 ±  7.592  ns/op
DefaultPortableReaderPerformanceTest.readPortableArray                    avgt  2000  2657.024 ± 27.831  ns/op

DefaultPortableReaderPerformanceTest.readPortableInt_nested               avgt  2000   199.158 ±  2.572  ns/op
DefaultPortableReaderPerformanceTest.readPortablePortableInt_nestedTwice  avgt  2000   354.254 ±  3.518  ns/op


@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2016

Here's the results of the performance tests showing no regression.
Query on a map with 1000 objects, equals predicate matching none of the objects.
1000 warmup iterations, 10 000 test iterations.

Master:
-------------------------

Benchmark                                                                 Mode    Cnt        Score      Error  Units
QueryPerformanceTest.query_object_equalsPredicate                         avgt  10000  571100.594 ± 3149.702  ns/op
QueryPerformanceTest.query_object_nested_equalsPredicate                  avgt  10000  581452.361 ± 3747.565  ns/op

QueryPerformanceTest.query_object_nested_collection_equalsPredicate       avgt  10000  566014.560 ± 3784.545  ns/op
QueryPerformanceTest.query_object_nestedTwice_collection_equalsPredicate  avgt  10000  561022.037 ± 4458.428  ns/op

QueryPerformanceTest.query_object_extractor_equalsPredicate               avgt  10000  571725.314 ± 4516.823  ns/op
QueryPerformanceTest.query_object_extractor_nested_equalsPredicate        avgt  10000  551384.820 ± 3458.459  ns/op

QueryPerformanceTest.query_portable_equalsPredicate                       avgt  10000  612568.149 ± 4890.245  ns/op
QueryPerformanceTest.query_portable_nested_equalsPredicate                avgt  10000 1191939.135 ± 8689.462  ns/op



This PR:
-------------------------

Benchmark                                                               Mode   Cnt        Score       Error  Units
QueryPerformanceTest.query_object_equalsPredicate                           avgt  10000   569755.182 ± 3648.758  ns/op
QueryPerformanceTest.query_object_nested_equalsPredicate                    avgt  10000   583469.226 ± 4032.534  ns/op

QueryPerformanceTest.query_object_nested_collection_equalsPredicate         avgt  10000   581612.533 ± 4863.558  ns/op
QueryPerformanceTest.query_object_nestedTwice_collection_equalsPredicate    avgt  10000   575616.807 ± 5169.231  ns/op

QueryPerformanceTest.query_object_extractor_equalsPredicate                 avgt  10000   602202.519 ± 6006.333  ns/op
QueryPerformanceTest.query_object_extractor_nested_equalsPredicate          avgt  10000   586909.376 ± 4431.454  ns/op

QueryPerformanceTest.query_portable_equalsPredicate                         avgt  10000    657033.492 ± 4135.005  ns/op
QueryPerformanceTest.query_portable_nested_equalsPredicate                  avgt  10000   1068649.211 ± 6064.044  ns/op


Additionally tests that could not be run on master:

QueryPerformanceTest.query_portable_extractor_equalsPredicate               avgt  10000   714809.491 ± 4468.231  ns/op
QueryPerformanceTest.query_portable_extractor_nested_equalsPredicate        avgt  10000   929088.360 ± 8312.178  ns/op

QueryPerformanceTest.query_portable_nested_collection_equalsPredicate       avgt  10000  1426056.094 ± 9750.003  ns/op
QueryPerformanceTest.query_portable_nestedTwice_collection_equalsPredicate  avgt  10000  1653886.417 ± 7966.638  ns/op
@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2016

@jerrinot @pveentjer @vbekiaris I've addressed all the comments. Could you have a look at the additional commits I've added and overall?

@jerrinot

This comment has been minimized.

Copy link
Contributor

commented May 12, 2016

+💯 because just 👍 is not enough! :) well done!

@vbekiaris

This comment has been minimized.

Copy link
Contributor

commented May 12, 2016

Great job! Code that is a pleasure to read, very well documented and tested. 👍

@pveentjer

This comment has been minimized.

Copy link
Member

commented May 13, 2016

👍

@pveentjer pveentjer merged commit 99da3fc into hazelcast:master May 13, 2016

1 check passed

default Build finished. 13700 tests run, 176 skipped, 0 failed.
Details
@tombujok

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2016

Thx to all reviewers!

@tombujok tombujok modified the milestones: 3.7, 3.6.3 May 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.