impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joe McDonnell (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4623: Enable file handle cache
Date Fri, 26 May 2017 22:44:45 GMT
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-4623: Enable file handle cache
......................................................................


Patch Set 22:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6478/22/be/src/runtime/disk-io-mgr-handle-cache.h
File be/src/runtime/disk-io-mgr-handle-cache.h:

Line 155:     uint8_t padding[CACHE_LINE_SIZE];
> What happened? The clang-tidy complaints seem to be about other types; does
I misunderstood and thought that changing the alignas(64) on this struct to a CacheLineAligned
would eliminate the problem, but aligning this in either way requires DiskIoMgr to also be
aligned.


http://gerrit.cloudera.org:8080/#/c/6478/22/be/src/runtime/disk-io-mgr.h
File be/src/runtime/disk-io-mgr.h:

Line 199: class DiskIoMgr {
> This is apparently a type that should be cache-aligned.
This cache alignment is required because FileHandleCachePartition is cache aligned. To pass
clang tidy, cache alignment should apply to both or neither. This upload does neither. I'm
switching it to both.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe5ff60971dd653c3b6a0e13928cfa9fc59d078d
Gerrit-PatchSet: 22
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message