CxxPreprocessor: collect missing includes conditionally

[ivangalkin]

• collect missing includes only if MissingIncludeFileCheck was activated
• don't collect successful includes - nobody uses them
• avoid memory run-time overhead and reduce memory footprint
• CxxParser::cxxpp - don't extend the lifetime of CxxPreprocessor artificially

Alternative (more radical) solution: #1638

---

This change is 

[guwirth]

@ivangalkin would like to merge this next. Don’t understand the WeakReference? Think garbage collection is allowed to remove object at any time. Means get() can also return null?

[ivangalkin]

@guwirth the idea is not to extend the lifetime of the CxxPreprocessor longer than necessary.

The "normal" reference (CxxParser::cxxpp) is never garbage collected, because it is static. The only way to perform its GC is to assign a null-pointer to the corresponding class field.

WeakReference doesn't increase the reference-count of the CxxPreprocessor. So it's just doesn't count in terms of GC. Other references do, and we have them in CxxSquidSensor::execute().

  public void CxxSquidSensor::execute(SensorContext context) {

    ...
    // create an AstScanner;
    // CxxAstScanner.creates() invokes  CxxParser.create() internally
    // that means, that the life-time of CxxPreprocessor is limited by the scope of
    // of CxxSquidSensor::execute()

    AstScanner<Grammar> scanner = CxxAstScanner.create(this.language, cxxConf,
      visitors.toArray(new SquidAstVisitor[visitors.size()])); 

    ...

    // now the same instance of the CxxPreprocessor is used to process all given files
    // after each file becomes processed it is analyzed by the visitors
    // among others there are two visitors, which call CxxParser's static methods
    // 1. CxxFileVisitor calls CxxParser.finishedParsing()
    // 2. MissingIncludeFileCheck calls CxxParser.getMissingIncludeFiles()
    // during these calls we still have the valid instance of CxxPreprocessor which is used
    // for these invocatoins
    scanner.scanFiles(files);

    ...
    // now the method CxxSquidSensor::execute() is over
    // the analysis of module is finished, we can destroy the preprocessor
}

As you see, there is no need to make the instance of preprocessor to the forewer-living singleton.
It's not that important for sonar-cxx itself, but after execution of our plugin is over, we should clean-up our resources. This might be crucial for the plugins, which are executed after us.

P.S. It would be more illegal to create the CxxParser on stack while execution of CxxSquidSensor::execute(). The reference could be passed to the 2 mentioned visitors while their instantiation. So there will be no need in static methods, static variables and all that complexity. However I didn't want to touch too much code.

[guwirth]

@ivangalkin thx for your explanation.

But I still think GC is allowed to remove it at any time. Article below says: Objects referenced by them may be garbage collected -- if the JVM runs short of memory -- even when they are being used.
http://www.programmr.com/blogs/what-every-java-developer-should-know-strong-and-weak-references

Means during scanFiles GC is allowed to remove CxxPreprocessor at any time?

[ivangalkin]

@guwirth the article describes the technique correct: if some instance is referenced by no strong reference, but only by WeakReferences -> object is garbage-collected. Some of the illustrations is in this article is artificial and misleading IMHO

WeakReference<String> wr = new WeakReference<String>(new String("abc"));

Such code means indeed, that developer cannot rely on the existence of the object. It might be garbage-collected at any time. There is no strong references.

Let me illustrate the work of weak references on the following example. Please try it here: https://ideone.com/k32xaP

/* package whatever; // don't place package name! */

import java.util.*;
import java.lang.*;
import java.io.*;
import java.lang.ref.*; 

/* Name of the class has to be "Main" only if the class is public. */
class Ideone
{
	public static class CxxPreprocessor {};
	public static void main (String[] args) throws java.lang.Exception
	{
		CxxPreprocessor strongRef = new CxxPreprocessor();
		WeakReference<CxxPreprocessor> weakRef = new WeakReference<CxxPreprocessor>(strongRef);
		// force garbage collection; the instance of CxxPreprocessor is referenced by
		// 1 strong reference
		// 1 weak reference
		// CxxPreprocessor is not gc'ed because there is at least one strong reference 
		System.gc();
		System.out.println("After 1st GC: " + weakRef.get());
		
		// remove strong reference and force the gc again
		// there is only a weak reference to the instance of CxxPreprocessor
		// CxxPreprocessor is garbage-collected
		strongRef = null;
		System.gc();
		System.out.println("After 2nd GC: " + weakRef.get());

	}
}

In our example (see my comment above) we have highly predictable lifetime. Visitors can call into the CxxPreprocessor only while it is existing. With the weak reference we 1) avoid leaks 2) create kind of run-time check, which prevents calls into the preprocessor at the wrong time.