Skip to content

Pipeline iteration builder#21

Open
Sanrag wants to merge 2 commits into
sladeware:masterfrom
Sanrag:pipeline_iteration_builder
Open

Pipeline iteration builder#21
Sanrag wants to merge 2 commits into
sladeware:masterfrom
Sanrag:pipeline_iteration_builder

Conversation

@Sanrag
Copy link
Copy Markdown
Contributor

@Sanrag Sanrag commented Sep 24, 2013

This allows for PipelineIteration instances to be constructed in a much more flexible way making it a general use pipeline framework.

This pull request is based on #17 and will go after it. Please only review Sanrag@71dcda9 for this pull request.

Make PipelineIteration less dependent on the specifics of the stage it
is executing.
There are small number of TODOs which would be cleaned up in a
subsequent change
This allows for PipelineIteration instances to be constructed in a much more flexible way making it a general use pipeline framework.
@matttproud
Copy link
Copy Markdown
Contributor

@Sanrag, I will give this a full review in a few days (trying to enjoy a post-employment computer detoxification), but I want to leave you with some interim thoughts:

import com.google.inject.*;

@Singleton
public class Main {
    // Guice creates an implicit Provider<T> for its injectables!  How nifty!
    private final Provider<Builder> builders;

    @Inject
    public Main(final Provider<Builder> builders) {
        this.builders = builders;
    }

    private void run() {
        final Builder builder = builder();

        System.out.println("Got builder " + builder);

        final Builder another = builder();

        System.out.println("Got another builder " + another);
    }

    public Builder builder() {
        return builders.get();
    }

    public static void main(final String... unused) {
        final Main main = Guice.createInjector(new Module()).getInstance(Main.class);

        main.run();
    }

    // NOOP module to demonstrate that Guice will do much of this for you—for better or for worse—
    // automagically.
    private static class Module extends AbstractModule {
        @Override
        protected void configure() {}
    }

    // Hard to know what happens with the scoping here.  Give it a try if you want to restrict one of
    // these suckers to a more constrained scope.
    private static class Builder {
        private final SingletonDependency singleton;

        @Inject
        public Builder(final SingletonDependency singleton) {
            this.singleton = singleton;
        }

        @Override
        public String toString() {
            return "Builder{" +
                    "singleton=" + singleton + " " +
                    "id=" + Integer.toHexString(System.identityHashCode(this)) +
                    '}';
        }
    }

    @Singleton
    public static class SingletonDependency {
        @Override
        public String toString() {
            return "SingletonDependency{" +
                    "id=" + Integer.toHexString(System.identityHashCode(this)) +
                    '}';
        }
    }

    // For your own tests …
    public static class ScopedDependency {
    }
}

I think you can effectively get away with the same behavior without passing around or exposing the injector to parts of the stack that ought not be aware of it. Most well-formed Guice implementations hide the injector from as much of the bowels as possible.

The example above yields:

Got builder Builder{singleton=SingletonDependency{id=9a082e2} id=8f0c85e}
Got another builder Builder{singleton=SingletonDependency{id=9a082e2} id=77f297e7}

@matttproud
Copy link
Copy Markdown
Contributor

@Sanrag, polite ping: Is this code review still alive? :)

@Sanrag
Copy link
Copy Markdown
Contributor Author

Sanrag commented Mar 26, 2014

We were having discussions around whether we want to make Groningen a general purpose tuning framework (a.k.a. Tunie) or keep it very specific to JVM Tuning. The discussion stretched and I kind of dropped the ball on this. If the decision is to go for latter, I will just go ahead and cancel the pull request.

@etheon
Copy link
Copy Markdown
Contributor

etheon commented Mar 26, 2014

I kind remember the discussion was whether to replace jvm tuning with a freeform pipeline or abstract the pipeline objects in order to allow the current Pipeline structure to be used as well as allowing the introduction of this freeform type.

My vote being for the latter since there is a significant amount of work that is done based on the nonBuilder style and the Builder style being pretty difficult to enforce any kind of ordering/enumeration/general “this is how the pipeline will behave”ness on...

On Mar 26, 2014, at 8:11 AM, Sanrag Sood notifications@github.com wrote:

We were having discussions around whether we want to make Groningen a general purpose tuning framework (a.k.a. Tunie) or keep it very specific to JVM Tuning. The discussion stretched and I kind of dropped the ball on this. If the decision is to go for latter, I will just go ahead and cancel the pull request.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants