Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13661 )
Change subject: KUDU-2381: Optimize DeltaMemStore for case of no matching deltas. ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@9 PS2, Line 9: Add a variable for lazy initialization to accelerate DeltaMemStore scan nit: no need to indent here http://gerrit.cloudera.org:8080/#/c/13661/2//COMMIT_MSG@11 PS2, Line 11: I ran a benchmark of this patch with the original workload mentioned in the JIRA (select * from my_wide_table where c1=<value that does not occur>). Here are the top CPU consumers before and after: Before: + 20.39% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DMSPreparerTraits>::Start(unsigned long, int) + 10.53% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch() + 8.64% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*) + 7.35% kudu-tserver kudu-tserver [.] LZ4_decompress_fast After: + 12.41% kudu-tserver kudu-tserver [.] kudu::tablet::CFileSet::Iterator::FinishBatch() + 10.74% kudu-tserver kudu-tserver [.] kudu::GetMemcmpableVarint64(kudu::Slice*, unsigned long*) + 9.94% kudu-tserver kudu-tserver [.] LZ4_decompress_fast + 7.54% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::AddDeltas(unsigned int, unsigned int) + 6.38% kudu-tserver kudu-tserver [.] bshuf_shuffle_bit_eightelem_AVX_avx2 + 6.03% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaFileIterator<(kudu::tablet::DeltaType)0>::PrepareBatch(unsigned long, int) ... + 3.21% kudu-tserver kudu-tserver [.] kudu::tablet::DeltaPreparer<kudu::tablet::DeltaFilePreparerTraits<(kudu::tablet::DeltaType)0> >::Start(unsigned long, int) ie your patch made DeltaPreparer::Start go from 20% CPU to 3% CPU. Great! I'll work on a fix for CFileSet::Iterator::FinishBatch Feel free to include the above in the commit message if you don't have time to run your own benchmark http://gerrit.cloudera.org:8080/#/c/13661/2/src/kudu/tablet/delta_store.cc File src/kudu/tablet/delta_store.cc: http://gerrit.cloudera.org:8080/#/c/13661/2/src/kudu/tablet/delta_store.cc@214 PS2, Line 214: CHECK(deleted_.empty()); nit: these lines are indented two extra spaces (we don't include the preprocessor ifdef for indentation) -- To view, visit http://gerrit.cloudera.org:8080/13661 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I799e81fef759d22f14d4530dbf0b250e0d7bc69b Gerrit-Change-Number: 13661 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Yao Xu <[email protected]> Gerrit-Reviewer: ZhangYao <[email protected]> Gerrit-Comment-Date: Tue, 18 Jun 2019 06:33:37 +0000 Gerrit-HasComments: Yes
