commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

Hi,

an intermediate commit for the bridge context.
The API should be easier to use.

I changed a small thing in the script API.
Now the InterpreterPool is not binded to a particular Document (as it used to be
with the new ConcreteInterpreterPool(Document)) - but it's the interpreter
itself which is binded - getInterpreter(Document, String).

This change easy the use of the BridgeContext as we don't have to specify the
document until the builder.build method. I also think it's better to have this
dependency there.

Christophe, any comments?
Hope it's ok, otherwise, we could go back or discuss about that.

Thierry.


-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Christophe Jolif <cj...@ilog.fr>]

Vincent Hardy wrote:
> 
> Thierry Kormann wrote:
> >
> > >       I strongly prefer automatic release with WeakReference!
> > >
> > >       There are a few examples of doing this already in Batik (the
> > > closest being the
> > > org.apache.batik.ext.awt.image.renderable.URLImageCache.  It may be
> > > more complex than you need, since it handles issues around the
> > > asynchronous loading of images (roughly equivalent to asynchronously
> > > instantiating the interpreter).
> >
> > I also prefer WeakReference.
> 
> So we are a happy family going with WeakReferences :-)

Ok.

-- 
Christophe

Re: commit and interpreter API changes

[Vincent Hardy <vh...@eng.sun.com>]

Thierry Kormann wrote:
> 
> >       I strongly prefer automatic release with WeakReference!
> >
> >       There are a few examples of doing this already in Batik (the
> > closest being the
> > org.apache.batik.ext.awt.image.renderable.URLImageCache.  It may be
> > more complex than you need, since it handles issues around the
> > asynchronous loading of images (roughly equivalent to asynchronously
> > instantiating the interpreter).
> 
> I also prefer WeakReference.

So we are a happy family going with WeakReferences :-)
V.

Re: commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

>       I strongly prefer automatic release with WeakReference!
> 
>       There are a few examples of doing this already in Batik (the
> closest being the
> org.apache.batik.ext.awt.image.renderable.URLImageCache.  It may be
> more complex than you need, since it handles issues around the
> asynchronous loading of images (roughly equivalent to asynchronously
> instantiating the interpreter).

I also prefer WeakReference. 

-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Thomas E Deweese <th...@kodak.com>]

>>>>> "CJ" == Christophe Jolif <cj...@ilog.fr> writes:

CJ> I pefectly understood your point of view.

CJ> What do you prefer:

CJ> - post initialization of InterpreterPool.

CJ> - InterpreterPool.getInterpreter(Document, String) with an
CJ>   explicit release method to be called when the document is no
CJ>   longer use.

CJ> - InterpreterPool.getInterpreter(Document, String) with an
CJ>   automatic release using WeakReference.

      I strongly prefer automatic release with WeakReference!

      There are a few examples of doing this already in Batik (the
closest being the
org.apache.batik.ext.awt.image.renderable.URLImageCache.  It may be
more complex than you need, since it handles issues around the
asynchronous loading of images (roughly equivalent to asynchronously
instantiating the interpreter).

Re: commit and interpreter API changes

[Christophe Jolif <cj...@ilog.fr>]

Thierry,

Thierry Kormann wrote:
> 
> > I will have the same kind of comments as previously: for me the bridge
> > event support is not the right place to manage the interperter sharing
> > mechanism for the same argument as previously: if someone wants to
> > access the interpreter oustide from the bridge (for example in a GUI for
> > debugging purposes) he will not have access to it.
> 
> OK so I will explain my really problem first.
> 
> With the current (or previous design), when we create a object for the bridge we
> have to specify each time the Document we are using.
> Ex:
> 
> new InterpreterPool(svgDocument);
> new BridgeContext(svgDocument);
> // ...
> builder.build(BridgeContext, svgDocument);
> 
> 3 times and users can easily have invalid states. My concern is to provide an
> 'easy to use' and 'safe' mecanism to use the bridge. I would like to specify the
> SVG document just one time (add everything should work).
>
> So, the only and last-in-the-process place I have to specify the document is on
> the build method of the GVT builder. That's why I would like to differ the use
> of the SVG document until there.
> 
> In conclusion, be able to create everything (including BridgeContext,
> InterpreterPool and whatever) without specifing the document.

I pefectly understood your point of view.

That's why the solution InterpreterPool.getInterpreter(Document, String)
was fine for me except that in this case it will be better to have a
release(Intepreter) method that is not currently defined/implemented (or
a mechanism using WeakReference to automatically remove the reference
when the document is no longer reachable except from the pool).
 
> Of course, the build method could then post-initialize the context with the
> document in use. For this I need an empty constructor on the InterpreterPool and
> a setSVGDocument on it is fine to me.
> 
> Otherwise, I could also forget this and let the user set many times the
> SVGDocument. It's up to you in fact...

What do you prefer:

- post initialization of InterpreterPool.
- InterpreterPool.getInterpreter(Document, String) with an explicit
release method to be called when the document is no longer use.
- InterpreterPool.getInterpreter(Document, String) with an automatic
release using WeakReference.

A+
-- 
Christophe

Re: commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

> I will have the same kind of comments as previously: for me the bridge
> event support is not the right place to manage the interperter sharing
> mechanism for the same argument as previously: if someone wants to
> access the interpreter oustide from the bridge (for example in a GUI for
> debugging purposes) he will not have access to it.

OK so I will explain my really problem first.

With the current (or previous design), when we create a object for the bridge we
have to specify each time the Document we are using.
Ex:

new InterpreterPool(svgDocument);
new BridgeContext(svgDocument);
// ...
builder.build(BridgeContext, svgDocument);

3 times and users can easily have invalid states. My concern is to provide an
'easy to use' and 'safe' mecanism to use the bridge. I would like to specify the
SVG document just one time (add everything should work).

So, the only and last-in-the-process place I have to specify the document is on
the build method of the GVT builder. That's why I would like to differ the use
of the SVG document until there.

In conclusion, be able to create everything (including BridgeContext,
InterpreterPool and whatever) without specifing the document.

Of course, the build method could then post-initialize the context with the
document in use. For this I need an empty constructor on the InterpreterPool and
a setSVGDocument on it is fine to me.

Otherwise, I could also forget this and let the user set many times the
SVGDocument. It's up to you in fact...

Any comment,
Thierry



-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Christophe Jolif <cj...@ilog.fr>]

Thierry Kormann wrote:
> 
> Thierry Kormann wrote:
> >
> > > How would that be done if the InterpreterPool is not linked to a
> > > particular document?
> >
> > link a particular instance of an interpreter to a particular document :)
> 
> By the way, a solution could be to have both an InterpreterFactory and an
> InterpreterPool.
> 
> InterpreterFactory abstracts the way to instanciate an Interpreter according to
> a particular text/script.
> 
> InterpreterPool provides a way to share Interpreter for a particular document.
> 
> And finally, what the BridgeContext needs is just the InterpreterFactory.
> The InterpreterPool can be used by the BridgeEventSupport.
> 
> Any comments?

I will have the same kind of comments as previously: for me the bridge
event support is not the right place to manage the interperter sharing
mechanism for the same argument as previously: if someone wants to
access the interpreter oustide from the bridge (for example in a GUI for
debugging purposes) he will not have access to it.

-- 
Christophe

Re: commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

Thierry Kormann wrote:
> 
> > How would that be done if the InterpreterPool is not linked to a
> > particular document?
> 
> link a particular instance of an interpreter to a particular document :)

By the way, a solution could be to have both an InterpreterFactory and an
InterpreterPool.

InterpreterFactory abstracts the way to instanciate an Interpreter according to
a particular text/script.

InterpreterPool provides a way to share Interpreter for a particular document.

And finally, what the BridgeContext needs is just the InterpreterFactory.
The InterpreterPool can be used by the BridgeEventSupport.

Any comments?
Thierry.

-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

> How would that be done if the InterpreterPool is not linked to a
> particular document?

link a particular instance of an interpreter to a particular document :)

Thierry.

-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Vincent Hardy <vh...@eng.sun.com>]

Thierry,

Thierry Kormann wrote:

> 1. I create an InterpreterPool (or choose the default one).
> 2. I choose a GraphicsNodeRenderContext (or use the default one).
> 3. I have my user agent (or use a predefined one)
> -> I can create as many BridgeContext as I want for as many documents as I want
> 
> So, that's why I prefer the InterpreterPool not be linked to a particular
> document (it's just a pool or a factory in my opinion).
> 
> I think that we could have the same behavior as before if in the
> BridgeEventSupport or whatever, you ask, the first time you find a script, an
> Interpreter and keep a reference on this one for future script elements.
> 
> I think it should be the responsability of the BridgeEventSupport to cache/reuse
> the same Interpreter if needed (not a build-in feature of the InterpreterPool).
> 
> At last, to let the Interpreter goes to GC, that will be possible when the
> BridgeEventSupport will release its listeners and other resources on the
> document.
> 
> Any comment?

I agree with the concern that you have to make the API simple for the
Batik 
user. However, I am not  too familiar with the InterpreterPool, but I 
would think that you need the notion of document somewhere, if only to 
find the handlers of specific events and make sure that the
'myECMAScript'
function in doc1 does not shadow the 'myECMAScript' function in doc2...

How would that be done if the InterpreterPool is not linked to a
particular document?

V.
> Thierry.
> 
> --
> Thierry Kormann
> email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
> Koala/Dyade/Bull @ INRIA - Sophia Antipolis
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: batik-dev-unsubscribe@xml.apache.org
> For additional commands, e-mail: batik-dev-help@xml.apache.org

Re: commit and interpreter API changes

[Christophe Jolif <cj...@ilog.fr>]

Thierry Kormann wrote:
> 
> > If you don't have objections I would preferer to let an external class
> > do the caching. I will try to work on it this week to remove the
> > problems introduced by the new API design.
> 
> Why not. Feel free to send my a proposal if you think that the current design
> sucks :)

No the design is ok! I will not change the API (or maybe add a release
method) I just want to modify the code to let it works...

-- 
Christophe

Re: commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

> If you don't have objections I would preferer to let an external class
> do the caching. I will try to work on it this week to remove the
> problems introduced by the new API design.

Why not. Feel free to send my a proposal if you think that the current design
sucks :)

Thierry


-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Christophe Jolif <cj...@ilog.fr>]

Thierry,

Thierry Kormann wrote:
> 
> > To sum up I think that the idea of changing this is interesting as it
> > seems to ease the use of the BridgeContext but it seems that to get a
> > real working solution, it requires a little more works than what have
> > been done since now. I not against this idea if you confirm me that it
> > helps a lot in BridgeContext.
> 
> Well, I really link the idea that the user can specify an InterpreterPool
> without considering a specific Document is nice.

I agree that it is nice.

> So the BridgeContext can take an InterpreterPool as a parameter and of course
> this pool he has defined can be reused for multiple documents. I am talking from
> a user point of view :)
> 
> 1. I create an InterpreterPool (or choose the default one).
> 2. I choose a GraphicsNodeRenderContext (or use the default one).
> 3. I have my user agent (or use a predefined one)
> -> I can create as many BridgeContext as I want for as many documents as I want
> 
> So, that's why I prefer the InterpreterPool not be linked to a particular
> document (it's just a pool or a factory in my opinion).
> 
> I think that we could have the same behavior as before if in the
> BridgeEventSupport or whatever, you ask, the first time you find a script, an
> Interpreter and keep a reference on this one for future script elements.
> 
> I think it should be the responsability of the BridgeEventSupport to cache/reuse
> the same Interpreter if needed (not a build-in feature of the InterpreterPool).

In this case we "break" the Pool pattern which will typically do the
chaching. Moreover if the user tries to ask for an interperter on the
pool from outside the bridge mechanism (in a GUI for debugging purpose
for example) he will not get the same interpreter...

If you don't have objections I would preferer to let an external class
do the caching. I will try to work on it this week to remove the
problems introduced by the new API design.

-- 
Christophe

Re: commit and interpreter API changes

[Thierry Kormann <Th...@sophia.inria.fr>]

> To sum up I think that the idea of changing this is interesting as it
> seems to ease the use of the BridgeContext but it seems that to get a
> real working solution, it requires a little more works than what have
> been done since now. I not against this idea if you confirm me that it
> helps a lot in BridgeContext.

Well, I really link the idea that the user can specify an InterpreterPool
without considering a specific Document is nice.

So the BridgeContext can take an InterpreterPool as a parameter and of course
this pool he has defined can be reused for multiple documents. I am talking from
a user point of view :)

1. I create an InterpreterPool (or choose the default one).
2. I choose a GraphicsNodeRenderContext (or use the default one).
3. I have my user agent (or use a predefined one)
-> I can create as many BridgeContext as I want for as many documents as I want

So, that's why I prefer the InterpreterPool not be linked to a particular
document (it's just a pool or a factory in my opinion).

I think that we could have the same behavior as before if in the
BridgeEventSupport or whatever, you ask, the first time you find a script, an
Interpreter and keep a reference on this one for future script elements.

I think it should be the responsability of the BridgeEventSupport to cache/reuse
the same Interpreter if needed (not a build-in feature of the InterpreterPool).

At last, to let the Interpreter goes to GC, that will be possible when the
BridgeEventSupport will release its listeners and other resources on the
document.

Any comment?
Thierry.

-- 
Thierry Kormann
email: Thierry.Kormann@sophia.inria.fr  http://www.inria.fr/koala/tkormann/
Koala/Dyade/Bull @ INRIA - Sophia Antipolis

Re: commit and interpreter API changes

[Christophe Jolif <cj...@ilog.fr>]

Thierry,

Thierry Kormann wrote:

> an intermediate commit for the bridge context.
> The API should be easier to use.
> 
> I changed a small thing in the script API.
> Now the InterpreterPool is not binded to a particular Document (as it used to be
> with the new ConcreteInterpreterPool(Document)) - but it's the interpreter
> itself which is binded - getInterpreter(Document, String).
> 
> This change easy the use of the BridgeContext as we don't have to specify the
> document until the builder.build method. I also think it's better to have this
> dependency there.
> 
> Christophe, any comments?
> Hope it's ok, otherwise, we could go back or discuss about that.

I renember myself hesitating between the two solutions, but honestly I
can't renember why I didn't choose the one you just commited (maybe
there was not strong reasons). So I will need to think a little bit more
about this before saying if the new API is ok or not for me. However
there's one thing I'm sure, it is that the implementation of the new API
as it is now may lead to some problems where the old one was ok.

For example with the new implementation if you call:

InterpreterPool mypool = new InterpreterPool();

Interpreter myint1 = mypool.getInterpreter(doc1, "text/ecmascript");
// some stuff
Interpreter myint2 = mypool.getInterpreter(doc2, "text/ecmascript");

myint2 will be the same interpreter as myint1 referencing the doc1
document 
not doc1 :-( Which is not what is intended I guess.

But I agree this can be fixed, this is not really linked to the API just
to how it is coded for now.

Hum, now I think I renember one of the reason why I choose the other
API.

With my solution:

1 document = 1 pool = potentially several interpreters

when the document is released, the pool & the interpreters are
automatically released (are available for GC).

With your solution:

1 pool = several documents  

=> when a document is released, the pool is not automatically released
(because used by other documents) => interpreters are not automatically
released (=> documents :-(). This means that if your solution is used we
should add something in the API to release the interpreters and we
should call this release method at the right moment. => need more work
to be done to correctly design this. 

However, It seems it is not a problem in your case because even if the
IntepreterPool is not linked to a document you create one per document
(per bridgecontext in fact).

To sum up I think that the idea of changing this is interesting as it
seems to ease the use of the BridgeContext but it seems that to get a
real working solution, it requires a little more works than what have
been done since now. I not against this idea if you confirm me that it
helps a lot in BridgeContext.

A+
-- 
Christophe