[PiranhaJava] Does not not clean up properly after removing feature flag

[NikitashP]

The un-referenced methods inside the code Piranha cleaned up is not removed. I was hoping the code which is no more referenced after clean up (dummyMethod in below example) will also be removed and hence a deep cleanup.

Before:

  public void bar() {
    if (expt.flagDisabled(TestExperimentName.SAMPLE_STALE_FLAG)) {
      System.out.println("Hi World");
      dummyMethod();
    }
  }

  private void dummyMethod() {
    System.out.println("Dummy Method");
  }

After:

  public void bar() {

  }

  private void dummyMethod() {
    System.out.println("Dummy Method");
  }

[mkr-plse]

When multi pass analysis is implemented (see here), this issue will be resolved.

Until then, to handle this specific problem, we can consider running Proguard before and after the application of Piranha, and remove the methods/fields that show up in the delta.

[NikitashP]

@mkr-plse Will unused variables caused due to the refactoring also be removed by this change ?

[mkr-plse]

When the two pass analysis is implemented, unused variables will also be handled. We don't have a timeline for the two pass analysis though.

[mevlanaayas]

@mkr-plse hi after reading this blog from uber https://eng.uber.com/piranha/ it says

Delete code that becomes unreachable due to performing the previous step. We refer to this as deep cleaning.

Does this quote mean piranha implements dep cleaning or I need to run tools like proguard? I think the question is related to this issue so I'm posting it here. thanks for help

[mkr-plse]

Other variants of Piranha (Swift, JS) have already implemented some form of deep cleaning. AFAIK, Proguard may be able to delete the deadcode from bytecode but not from source. We expect to add this feature to PiranhaJava. Unfortunately, we haven't yet gotten to this milestone yet.

[himanshuguptan]

@NikitashP Looks like the milestone hasn't been implemented yet. What workaround did you choose to enable such refactoring?

[NikitashP]

After the Feature Flag clean-up, another round of dead code clean-up using ErrorProne works. The clean up is not Targetting "only" the Feature Flag clean-up left overs, that is an issue. In the end with or without Feature Flag clean up you don't want dead code on master so can justify it that way.

-Xplugin:ErrorProne -XepPatchChecks:UnusedMethod,UnusedNestedClass,UnusedAnonymousClass,RemoveUnusedImports,DeadException,DeadThread,UnusedCollectionModifiedInPlace -XepPatchLocation:IN_PLACE

[mkr-plse]

@NikitashP We are working on making this much easier with a newer implementation of Piranha. See here. Please check it out as we expect it to be very customizable and adaptable for various use cases. cc @ketkarameya