[jira] [Commented] (IMPALA-9373) Trial run of IWYU on codebase

["ASF subversion and git services (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IMPALA-9373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17066816#comment-17066816 ] 

ASF subversion and git services commented on IMPALA-9373:
---------------------------------------------------------

Commit da5b498c18ba1a22b122682da73481af633a3398 in impala's branch refs/heads/master from Tim Armstrong
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=da5b498 ]

IMPALA-9373: more tactical IWYU fixes

This is a grab-bag of fixes that I did with a mix of manual
inspection. The techniques used were:
* Getting preprocessor output for a few files by modifying
  command lines from compiler_commands.json to include -E.
  This is revealing because you see all the random unrelated
  cruft that gets pulled in. A useful one liner to extract
  an (approximate) list of headers from preprocessor output is:
  grep '^#.*h' be/src/util/CMakeFiles/Util.dir/os-info.cc.i | \
      grep -o '".*"' | sort -u
* Looking at the IWYU recommendations for guidance on what
  headers can be removed (and what need to be added).
* Grepping for includes of headers, especially in other headers
  where they become viral. An example one-liner to find these:
  git grep -l 'include.*<iostream>' | grep '\.h$'

Non-exhaustive list of changes made:
-----------------------------------
Unnest classes from TmpFileMgr so we can forward-declare them.
This lets us remove tmp-file-mgr.h from buffer-pool.h and
query-state.h, which are both widely included headers in the
codebase.

Also remove webserver.h from other headers, since it
pulls in openssl-util.h and consequently a lot of
openssl headers.

Avoid including runtime/multi-precision.h in other headers.
It pulls in a lot of boost multiprecision headers that
are only needed for internal implementations of math
and decimal operations. This required replacing some
references to int128_t with __int128_t, which I don't
think significantly hurts code readability.

Also remove references to decimal-util.h where they're
not needed, since it transitively pulls in
multi-precision.h

Reduce includes of boost/date_time modules, which are
transitively many places via timestamp-value.h.

Remove transitive dependencies of timestamp-value.h
to avoid pulling in remaining boost date_time headers
where not needed. Dependent headers are:
scalar-expr-evaluator.h, expr-value.h

Remove references to debug-util.h in other headers,
because it pulls in a lot of thread headers.

Remove references to llvm-codegen.h where possible,
because it pulls in many llvm headers.

Other opportunities:
--------------------
* boost/algorithm/string.hpp includes many string algorithms
  and pulls in a lot of headers.
* util/string-parser.h is a giant header with many dependencies.
* There's lots of redundancy between boost and standard c++
  headers. Both pull in vast numbers of utility headers for
  C++ metaprogramming and similar things. If we reduced virality
  of boost headers this would help a lot, and also if we switch
  to equivalent standard headers where possible (e.g. unordered_map,
  unordered_set, function, bind, etc).

Compile time with clang/ASAN:
-----------------------------
Before:
real    9m6.311s
user    62m25.006s
sys     2m44.798s

After:
real    8m17.073s
user    55m38.425s
sys     2m25.808s

Change-Id: I8de71866bdf3211e53560d9bfe930e7657c4d7f1
Reviewed-on: http://gerrit.cloudera.org:8080/15248
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


> Trial run of IWYU on codebase
> -----------------------------
>
>                 Key: IMPALA-9373
>                 URL: https://issues.apache.org/jira/browse/IMPALA-9373
>             Project: IMPALA
>          Issue Type: Sub-task
>          Components: Infrastructure
>            Reporter: Tim Armstrong
>            Assignee: Tim Armstrong
>            Priority: Major
>
> I did a trial run and implemented some of the suggestions of IWYU to confirm that it made sense. I'll post a patch with the changes and leave notes here.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org