impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tianyi Wang (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-6121: remove I/O mgr request context cache
Date Wed, 01 Nov 2017 21:47:58 GMT
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/8408 )

Change subject: IMPALA-6121: remove I/O mgr request context cache
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8408/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8408/4//COMMIT_MSG@12
PS4, Line 12: from TCMalloc's thread caches should scale much better than a
Though reader_context_ is not frequently allocated in current code, theoretically for reducing
allocation won't it be better to store DiskIoRequestContext directly (or boost::optional<>
for a "nullable cell")?


http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-reader-context.h
File be/src/runtime/disk-io-mgr-reader-context.h:

http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-reader-context.h@274
PS4, Line 274:       __sync_synchronize();
Just a question - Does b need to be atomic here? If it does not generate a MOV then this __sync_synchronize
might not be effective as well


http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-reader-context.h@319
PS4, Line 319:       if (!is_on_queue_ && num_threads_in_op_.Load() == 0 &&
!done_) {
The return value of Add can be used to remove this Load


http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-test.cc
File be/src/runtime/disk-io-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-test.cc@251
PS4, Line 251:   unique_ptr<DiskIoRequestContext> writer = io_mgr.RegisterContext(NULL);
nullptr?



-- 
To view, visit http://gerrit.cloudera.org:8080/8408
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108
Gerrit-Change-Number: 8408
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <twang@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Nov 2017 21:47:58 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message