Don't log linking build actions by default.

[bruntib]

The logger can be given a flag if the user wants to keep the linking
actions in the compilation database.

[csordasmarton]

Parameter names should end with underscore: filename_

[csordasmarton]

I think you should remove the blocks from here and below.

[csordasmarton]

Finds, element

[csordasmarton]

Mark the end of the sentence with dot: ..

[csordasmarton]

Is it necessary to check this condition. If it is true we set it to true?

[whisperity]

How is executing CodeChecker log --build "g++ a.cpp" handled now? Will there be a compilation action in the JSON, and if keep linkage is set, two commands?

[gyorb]

Please update the feature based on the discussion. In the issue.

[gyorb]

@whisperity the idea is that two separate json files will be generated (compilation, linking). If we can not decide right now if it is a linking only action I would still add it to the compilation actions, so we wont loose files which should be analyzed.

[bruntib]

I'm in trouble a bit how this should be implemented. The goal was to omit linking actions from the compilation database, because these are

1. not used by the checkers
2. resulting too big files (>2GB in some cases) which can't be read by the Python scripts,
3. not part of the compilation database format (I don't remember who mentioned this and what did he base it on).

On the other hand if we want to save them then the decision was to place them in another file (#1436). This doesn't fit in the current implementation of the build logger, since it is a general C/C++/Java logger, the log collector part can't distinguish between compilation and linking. And the linking actions can't be skipped in CodeChecker, because it's too late (see item 2.)
So my questions are: do we really need linking actions and if yes, then should this be implemented in the logger somehow?

[whisperity]

In case CTU ever attempts to go beyond considering an ambiguous function call due to multiple implementations in the mangled name map as unknown, in some way, linkage information will be needed, just as how the clustering module in CodeCompass also requires them.

Until then, it looks safe enough to omit, as nothing currently understands them in any useful manner.

[whisperity]

If I am taking it correctly the problem with too big JSONs is that we first load the entire JSON array into the memory and then try to iterate on it.

Could this be changed into a more dynamic, stream like approach, where the JSON is processed without it being fully loaded at any given time, and thus we can manually ignore link commands and only store build actions in the memory that are relevant? (Or perhaps store even less in memory!)

[dkrupp]

I don't really understand @bruntib's 3rd concern here. In this pull request you distinguish between linker and compliation commands based on the extension of the source file. Isn't this good enough? Why can't you log always linking actions to linking_actions.json and keep only compilation commands in compile_commands.json? So I suggest removing CC_LOGGER_KEEP_LINK env var from ldlogger-tool-gcc.c <https://github.com/Ericsson/codechecker/pull/1450/files#diff-a111e62f6311348903e2dbb2cb1e1a91> and remove --keep-link parameter from codechecker. BTW compilation database format specification is described here https://clang.llvm.org/docs/JSONCompilationDatabase.html with this definition: "A compilation database is a JSON file, which consist of an array of “command objects”, where each command object specifies one way a translation unit is compiled in the project. " Stream processin of compila_commands.json is a good idea besides this PR.

…

On Fri, Mar 16, 2018 at 11:48 PM, Whisperity ***@***.***> wrote: If I am taking it correctly the problem with too big JSONs is that we first load the entire JSON array into the memory and then try to iterate on it. Could this be changed into a more dynamic, stream like approach, where the JSON is processed without it being fully loaded at any given time, and thus we can manually ignore link commands and only store build actions in the memory that are relevant? (Or perhaps store even less in memory!) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHqK6WV9iznP3dQjFPRi4JdqWgb0auX8ks5tfEFOgaJpZM4SmVjw> .

[whisperity]

@dkrupp It is not always trivial if a compile command is a simple compile command or also a link command. You are in luck with CMake as it, in most cases, generates separate compile and link commands (then again, CMake can also natively export a CCDB, so there is no reason to log the build at all), but older/custom/different systems can end up having their build logged like this:

/usr/bin/g++-5 /home/whisperity/Dummy/main.cpp

This is a compile and a link command. So the logger tool must either be able to see the ld (gold? lld?) calls "inside", or manually parse the arguments and try to figure out if it is a compilation only or a link only or a "both" command.

[Xazax-hun]

In case we are not afraid of adding additional dependencies, there are som libraries for lazy json parsing which we could try:
https://github.com/fenhl/lazyjson
https://pypi.python.org/pypi/ijson/

I think it is a good idea to both remove linking commands and make handling of potentially large files lazy.

[bruntib]

My problem was not related to distinction of linking and compilation commands. The problem is that the only .json output file has to be given to the CodeChecker log command by -o flag. So one option would be to introduce another output flag for linking actions, or another is using a "link_" prefix which is prepended to the output file name. Any option we choose, the current build logger implementation doesn't support several output files based on such a language specific build actions like compilation/linking, since the build logger logs Java actions too.

So for now I extend the build logger so it can be able to write two output files. The details can be specified later.

[whisperity]

For now we can always say that in case of Java, the link command list will be empty, because Java always does dynamic linking and classloads. 😬 (Not that anyone would actively be using the ld-logger anyways.)

[dkrupp]

Verified on xerces with or without logging linking.
compile_commands_with_linking.json 8.7MB
compile_commands_no_linking.json 447KB
impressive decrease!

Number of build actions analyzed 631 in both cases.
Looks good to me.