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

Performance regression in BaseIndexStore #11280

Closed
dsukhoroslov opened this issue Sep 3, 2017 · 6 comments
Closed

Performance regression in BaseIndexStore #11280

dsukhoroslov opened this issue Sep 3, 2017 · 6 comments

Comments

@dsukhoroslov
Copy link
Contributor

@dsukhoroslov dsukhoroslov commented Sep 3, 2017

Current implementation of BaseIndexStore.copyToMultiResultSet(..) is:

    final void copyToMultiResultSet(MultiResultSet resultSet, Map<Data, QueryableEntry> records) {
        resultSet.addResultSet(new HashMap<Data, QueryableEntry>(records));
    }

is it really necessary to create a new Map here? I see a serious bottleneck in this line. After changing it to:

    final void copyToMultiResultSet(MultiResultSet resultSet, Map<Data, QueryableEntry> records) {
    	resultSet.addResultSet(records);
    }

I see performance improve in ~2.5 times on range queries over indexed attribute. All tests are still passed, and I don't see any issues in HZ behaviour.

Please have a look.

Thanks, Denis.

@tombujok tombujok self-assigned this Sep 3, 2017
@tombujok tombujok added this to the Backlog milestone Sep 3, 2017
@tombujok
Copy link
Contributor

@tombujok tombujok commented Oct 13, 2017

The regression is caused by this correctness fix:
#9688

@tombujok tombujok modified the milestones: Backlog, 3.9.1 Oct 13, 2017
@tombujok
Copy link
Contributor

@tombujok tombujok commented Oct 13, 2017

Will be addressed in 3.8.7 and 3.9.1

@tombujok tombujok changed the title Performance improve in BaseIndexStore Performance regression in BaseIndexStore Oct 13, 2017
@dsukhoroslov
Copy link
Contributor Author

@dsukhoroslov dsukhoroslov commented Oct 13, 2017

Thanks @tombujok,
BTW, do you have unit tests where network partitioning/partition migration cases tested? I'd like to add this kind of tests to my test suite..

@mdogan
Copy link
Contributor

@mdogan mdogan commented Oct 13, 2017

QueryBounceTest is to test query with and without index while bouncing members. So, it tests effects of member shutdown/restart and partition migrations.

IndexSplitBrainTest is to test indexed query during network partition.

@tombujok tombujok modified the milestones: 3.9.1, 3.8.7 Oct 25, 2017
@tombujok
Copy link
Contributor

@tombujok tombujok commented Nov 3, 2017

Fixed via #11705

The fixes enables the users to choose: performance vs. correctness.

Will be released in 3.8.7, 3.9.1 & 3.10

@tombujok tombujok closed this Nov 3, 2017
@dsukhoroslov
Copy link
Contributor Author

@dsukhoroslov dsukhoroslov commented Nov 15, 2017

Hi @tombujok,
I just tried this new option with HZ 3.8.7. Tried all 3 values. With COPY_ON_READ I see no performance improvement, it works like it was in HZ 3.8.6. With NEVER the performance for reads increased in 2..3 times, very good, but not sure this value can be used safely. With COPY_ON_WRITE I see the same improve for reads, but there is huge performance degradation on inserts, they become 30..50 times slower. Have a look, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.