Bulk file check on slaves

[artms]

When phabricator jenkins plugin does coverage file parse it checks each covered file on slave which source directory contains source file this can be expensive if you have large code base or large amount if covered source directories or when you have large latency between master<->slave (on another coast), this actually happened with one of project where such "check" was taking 1.5h to completed.
Instead of checking individual files - we collect all needed files and make bulk request. PathResolverChooseMultiCallable is serialized by jenkins and sent to be executed on slave which does this efficiently and returns back result. This might cause extreme memory usage for extremely large code base if such case happens - bulk request will have to be chunked

[coveralls]

Coverage increased (+0.2%) to 90.799% when pulling ad43cd2 on artms:bulk_file_check into 6cfd526 on uber:master.

[kageiit]

We can actually make this lot more efficient

Phab already knows the files affected as part of the diff. In the same spirit as #151

We can just collect file names from whats modified and just check state on those files and ignore the rest of the files in the coverage report. That will mitigate any large memory concerns as well

[artms]

@kageiit - if we check only files modified won't we miss files which were covered by new tests added into code base? Eg. if I simply add add new unit test which tests some real code but not changing that code itself - we will not even calculate line coverage?

[kageiit]

We dont need to calculate the coverage for the main file. We just need aggregates to report back which we can still do without looking if the file exists. Processing line coverage for every line will still be useless as we cannot show it in the ui

[msridhar]

Drive-by comment: from what I've read recently, either ArrayList or ArrayDeque is almost always better than LinkedList; this is now checked by ErrorProne

[artms]

Thanks, fixed

[coveralls]

Coverage increased (+0.2%) to 90.799% when pulling 8fc0ff7 on artms:bulk_file_check into 6cfd526 on uber:master.

[kageiit]

can replace this for loop with for(fileName: fileNames)

[artms]

We need xml nodes:

            // Loop over all files in the coverage report
            for (int i = 0; i < classes.getLength(); i++) {
                Node classNode = classes.item(i);
                String fileName = classNode.getAttributes().getNamedItem(NODE_FILENAME).getTextContent();

                if (includeFileNames != null && !includeFileNames.contains(FilenameUtils.getName(fileName))) {
                    continue;
                }

                fileName = join(detectedSourceRoots.get(fileName), fileName);

                SortedMap<Integer, Integer> hitCounts = internalCounts.get(fileName);
                if (hitCounts == null) {
                    hitCounts = new TreeMap<Integer, Integer>();
                }

                NodeList children = classNode.getChildNodes();

Can you be more specific?

[kageiit]

im saying we are looping twice over the same things. There is opportunity to DRY something.

SInce we already know filenames we care about from previous loop, we dont have to repeat the same logic again

[artms]

@kageiit - now I'm not looping again through same loop although it requires us to collect childNodes for this loop to work

[coveralls]

Coverage increased (+0.2%) to 90.793% when pulling 8a2362c on artms:bulk_file_check into 6cfd526 on uber:master.