[BEAM-22] Remove unneccesary Fn cloning

[tgroh]

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

• Make sure the PR title is formatted like:
  [BEAM-<Jira issue #>] Description of pull request
• Make sure tests pass via mvn clean verify. (Even better, enable
  Travis-CI on your fork and ensure the whole test matrix passes).
• Replace <Jira issue #> in the title with the actual Jira issue
  number, if there is one.
• If this contribution is large, please file an Apache
  Individual Contributor License Agreement.

---

The ParDoInProcessEvaluator is provided clones of a DoFn when
appropriate, and should not serialize them.

[tgroh]

The ThreadLocal-cloning Fn Factories were added in 483a5a4 but cloning was not removed (as appropriate) in that CR.

R: @kennknowles @bjchambers

[tgroh]

The test failures are because TestDoFn does not permit itself to be reused (specifically, if startBundle is called after any other DoFn method is called, including finishBundle, it will fail an assertion).

There are two potential fixes. My preference would be to modify TestDoFn to be reusable, so long as the call sequence is correct (e.g. startBundle resets the state if state is FINISHED). The other is to use the ThreadLocal only when startBundle and finishBundle are the defaults (i.e. the declaring class is org.apache.beam.sdk.transforms.DoFn

This may be a discussion suitable for the mailing list. Thoughts?

[bjchambers]

Have the tests been updated now? Should this be passing now? Also, we should state that this isn't improper cloning, but excessive cloning. It is still valid according to the model to clone these.

[tgroh]

Updated the commit message; The RunnableOnService tests all pass, but a future-change that replaces the DirectRunner in its entirety does not, due to the implementation of Write

[kennknowles]

LGTM but I will wait until tomorrow to merge, in case there is any last-minute feedback on BEAM-38.

[kennknowles]

Based on the dev list discussion, it looks like DoFn reuse might already be prevalent, so there's no downside here. Merging.