[jsword-devel] Activator classes across the JSword library + Refactoring for JS-109

[DM Smith]

This is a great idea. Only gotcha is the construction of a parallel view.

In Him,
	DM

On Aug 23, 2012, at 5:44 PM, Chris Burrell <chris at burrell.me.uk> wrote:

> For those who want the details of the analysis so far:
> 
> We start with AbstractPassageBook.getOsisIterator. This core function essentially obtains the XML and converts it to a XML structure.
> This iterates over ranges first
> within each range, (1)
> adds some headers to the xml document (2)
> iterates over each verse (3)
>  calls getRawText
> getRawText in turn calls the relevant backend
> The backend, calls activate
> uses the stored fields to retrieve the random access files that have already been opened (and cause the problem)
> returns OSIS in raw "string" form (4)
> adds the xml to the xml document (5)
> My suggested refactoring, would be to re-jig things as follows:
> (removed)
> (removed)
> (removed)
> (removed)
>  calls getRawText
> getRawText in turn calls the relevant backend
> The backend, calls activate (? no longer required?)
> uses the stored fields to retrieve the random access files that have already been opened (and cause the problem)
> within each range, (1)
> use passed-in parameter and adds some headers to the xml document (2)
> iterates over each verse (3)
> returns OSIS in raw "string" form (4)
> adds the xml to the xml document (5)
> 
> Something along those lines anyway (lines in bold have moved/changed slightly)
> Chris
> 
> 
> 
> On 23 August 2012 22:33, Chris Burrell <chris at burrell.me.uk> wrote:
> Hi all
> 
> Just wondering if this section of the "Activator" code was introduced to reduce memory as a result of observing an issue, or not? I'm looking at SwordGenBook in particular, but seeing all kinds of interesting things
> 
> Activator has a reduceMemoryUsage method. This is based on KILL. Of the 3 enum constants, only 1 has a method that actually does something. And as far as I can tell, anyone can call reduceMemoryUsage at any time, which will then thoroughly break anything in the process of running by nulling out a whole load of references.
> Secondly, there is this Lock object. It's not used to synchronise on, nor used in any form. The private javadoc comment is slightly mis-leading, unless I'm missing a piece of code somewhere.
> SwordGenBook.activate gets an empty set from its parent, and thereby the loop doesn't do anything.
> the "global" field is always an empty ReadOnlyList as a result, can we remove and simplify this?
> In the SwordBook:
> activate() does nothing (apart from registering it with the activator, which seems to defeat the point as there is no clean-up)
> deactivate() deactivates the backend it is based upon. Should this be done preferably by the backend rather than here?
> Finally, in order to address JS-109 and contain the refactoring process, I am thinking of changing the method signature for AbstractPassageBook.getRawText(). My intended approach is to pass in an extract parameter, as 1 or 2 anonymous class to post-process what has been discovered in reading the raw data. By doing so, I believe I might be able to push both loops from AbstractPassageBook into the backends, so that the backends themselves deal with iterating through ranges and verses. 
> 
> The idea being, for performance reasons, we want to keep the random access files open and re-use the state. However, we don't want to be sharing the state across threads, and therefore that state needs to be local. Not sure if it's going to be feasible doing it that way, but I thought I might give it a go. It obviously might mean re-jigging the vast majority of the backends. Shout if you want some more details.
> 
> Cheers
> Chris
> 
> 
> _______________________________________________
> jsword-devel mailing list
> jsword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/jsword-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/jsword-devel/attachments/20120825/eb9ee1a9/attachment.html>