how to replace the implementation of IOUtil on runtime?

[j.r.]

To get a good performance for Concordion.NET based on Concordion and IKVM (https://groups.google.com/forum/#!topic/concordion/uZfO-k0Jmww), it is necessary to provide a C#/.NET implementation of IOUtil. Thus, a mechanism is required to replace the implementation on runtime. What about using the singleton pattern as first, simple approach? In the startup of Concordion.NET tests the singleton reference of IOUtil could be changed to point to the .NET implementation.
Do you think this could be a workable?

[Tim Wright]

So we'd really be creating a FileSystemAdaptor object that calls the IOUtils methods (or other methods as necessary). Makes sense to me. And it's cleaner than calling static methods in IOUtils.

However, I'm not a big fan of singletons. Is there any way we can have a FileSystemAdaptor set in the ConcordionBuilder? Or something else like that?

Tim

On 28 January 2016 at 04:44, j.r. <ratzing...@gmail.com> wrote:

To get a good performance for Concordion.NET based on Concordion and IKVM (https://groups.google.com/forum/#!topic/concordion/uZfO-k0Jmww), it is necessary to provide a C#/.NET implementation of IOUtil. Thus, a mechanism is required to replace the implementation on runtime. What about using the singleton pattern as first, simple approach? In the startup of Concordion.NET tests the singleton reference of IOUtil could be changed to point to the .NET implementation.
Do you think this could be a workable?

--
You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.
To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/36c7f40a-8d3b-40ee-a6ac-b1cff36a995b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Tim

021 251 5593

http://www.linkedin.com/in/drtimwright 

[j.r.]

I am am not sure what you mean by FileSystemAdaptor.

In a first simple step, I would derive from the IOUtil class and override the implementation with a platform specific one. The greatest performance improvement can be gained by replacing the loading of Resources. Therefore the IOUtil of Concordion.NET uses a .NET specific mechanism - this loads the resources from the dll instead of the jar file:
public override string readResourceAsString(string resourcePath)
        {
            var resourceManager = new ResourceManager("Concordion.NET.HtmlFramework", typeof(IOUtil).Assembly);
            var resourceName = resourcePath.Substring(resourcePath.LastIndexOf('/') + 1);
            var resourceAsString = resourceManager.GetString(resourceName);
            var result = resourceAsString.Replace("\r", "");
            return result;
        }

To be able to use this .NET specific implementation in the ikvmc compiled Java classes of Concordion, we need a mechanism to switch implementations on runtime.
My first approach was also to create another method in the ConcordionExtender / ConcordionBuilder such as withIOUtil(). But this had the disadvantage that the ConcordionBuilder has to pass the IOUtil instance to all classes that need it. This creates changes to many interfaces of class for the simple replacement task of injecting a .NET specific implementation of an util class.
As a result, I would like to try a singleton / factory / etc. approach instead. I would opt for the singleton because it is a very small change - just calling IOUtil.instance.<method>() instead of IOUtil.<method>

[Nigel Charman]

As I understand it, Tim's suggestion would be a FileSystemAdapter interface that we might implement with JvmFileSystemAdapter and DotNetFileSystemAdapter classes (let me know if I'm wrong Tim!).

Thinking about it further, we already have Source and Target interfaces that isolate the reading and writing for us. IOUtils is used by the ClassPathSource and FileTarget implementations of these interfaces. However IOUtil also "leaks" I/O activity into ConcordionBuilder (for reading CSS and JavaScript resources), ThrowableRenderer (for reading the visibility toggler javascript) and XMLSpecificationReader (in Concordion 2.0 for printing the converted HTML as a string).

My suggestion would be to move the functionality from IOUtil into the Source and Target interfaces and implementations, and pass these to the classes that use IOUtils currently. We can then deprecate or remove IOUtil in Concordion 2.0. It's in an internal package, and I can't see it used in any extensions.

While this is a bigger change than your suggestion, it shouldn't be too much change, and we already have a few breaking interface changes for Concordion 2.0.

Nigel.

--
You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.

To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/765ecf5a-ba74-41f5-89f3-bdcdb1f68db5%40googlegroups.com.

[Tim Wright]

What Nigel said :)

Tim
021 251 5593
Sent from phone.

To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/56AA59DF.7010900%40gmail.com.

[j.r.]

Tim, Nigel, thank you for your explanation and for your great ideas how to improve Concordion.

Could we address one topic after the other?

I recognize that I did not explain my problem well – sorry that’s my fault. It was misleading from my side to write “replace IOUtil on runtime” in the headline of this thread so that you had the impression that the problem I am trying to solve is related to file system operations. I apologize for that.

What is the problem I am trying to solve?

The loading of resources (embedded.css and visibility-toggler.js) through the classLoader.getResourceAsStream is too slow on the .NET platform. Nigel, as you explained this is used in ConcordionBuilder and ThrowableRenderer. My approach to solve this problem would be to provide a C# implementation for the IOUtil.readResourceAsString() method.

To make the handling of resources explicit, we could pull out the functionality from IOUtil into some sort of ResourceLoader or ResourceManager. Usally, the Java implementation based on classLoader.getResourceAsStream would be used when Concordion is run on the Java platform. When the ikvmc compiled Concordion classes are executed on the .NET platform, we need a mechanism to instantiate the C# implementation and hand it over to the Concordion classes. What's the simplest thing that could possibly work?

Is the problem I am trying to solve a little bit clearer?

[Nigel Charman]

Hi Jacek

I think I understand the problem, except I don't understand how the "mechanism to instantiate the C# implementation and hand it over to the Concordion classes" would work.

Since we already have the Source and Target interfaces and classes, those would be the right classes to move the functionality too and make it extensible. I'm happy to make the change to the Java code to support this. Would you be able to instantiate the C# implementation from either:

1) Alternative implementations of the Source and Target interfaces, or
2) Extensions of ClassPathSource and FileTarget?

It should then be a question of how we wire these up. ConcordionBuilder already has withSource() and withTarget() methods.

Does this work for you? Sorry I haven't had time to look at the code to see how this implementation is working yet.

Nigel.

--
You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.

To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/1ad14c56-96a5-49ab-8a5d-2558038b2793%40googlegroups.com.

[Tim Wright]

Hey,

I think we actually agree on a lot:

* We need to abstract file system operations so that concordion.NET can use a more optimised set of operations for performance reasons.

We just disagree on:

* Should we make IOUtil an object we pass around (via the Source and Target interfaces) or make it a singleton and provide a factory method to get the current set of operations.

To be honest, I'm a bit torn. However, we should do at least this:

* Make the methods in IOUtil non-static.

* Extract an interface

* Use the interface explicitly in the code

Then we just need to decide if we want to pass the IOUtils object around or get it where needed by a factory method (like IOUtil.getInstance - or whatever other factory method we need).

How about we just get it using a factory method as this is less initial work. Then once that's done we can investigate replacing the Source and Target interfaces.

I just did a bit of code analysis and the IOUtil class is used in quite a few places - so isolating the code into Source and Target might be a bit tricky.

Tim

 

To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/56AB29CC.1000408%40gmail.com.

For more options, visit https://groups.google.com/d/optout.

[j.r.]

Hey Nigel, Hey Tim,

Yes, we agree on a lot.

Thank you for your additional explanation.

At least it helped to lift the fog in my brain a bit. :o)

Yes, we should make the methods for loading the resources embedded.css and visibility-toggler.js non static.

If you make the changes to the Java code, I am happy with any solution for dependency injection (either by passing instances around or with a factory method).

[Tim Wright]

I'll create a pull request for this. It shouldn't be too hard.

Tim

--

You received this message because you are subscribed to the Google Groups "concordion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to concordion-de...@googlegroups.com.
To post to this group, send email to concord...@googlegroups.com.
Visit this group at https://groups.google.com/group/concordion-dev.

To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/7e9a5c8f-b856-4904-b2b0-470de4f17165%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Nigel Charman]

Beat you to it! https://github.com/concordion/concordion/pull/158

To view this discussion on the web, visit https://groups.google.com/d/msgid/concordion-dev/CAJifTypjMODYM_0OjJ90DWf8RQz%3DD2dz-UZYc%2B0voo1t6ee24w%40mail.gmail.com.

[j.r.]

If we go with the singleton, then the code already exists :-)

https://github.com/shakaree/concordion/tree/ikvm-native-IOUtil