Avik Sengupta wrote:

>Ok, first up, sorry if my mail was too bad tempered .. probably a result of a 
>hangover from drinking a bottle of Remy Martin on saturday night... i didnt 
>take that personally.. i still love you ;-)
>  
>
I understand.. . been there.  Thanks man...back at ya.

>Having said that, i still think i am right and you were wrong :)
>
>1. Synchronisation in Formula Parser will go. Agreed. As i said, its an old 
>decision before i knew how usermodel works. 
>  
>
cool

>2. Thread Safety of POI should be as follows: "Use of one instance of workbook 
>accross different threads is not supported, however, it is safe to create 
>different instances of workbooks in different threads. "
>  
>
Of course.  That is the definition of *not thread safe* meaning you must 
roll your own.

>I believe that was how POI was, and still is. If I am wrong on this ( as in, 
>its unsafe to use 2 workbooks in two threads) then we should a) mention it 
>prominently on our web page and b) work to change that, since servlet use 
>cannot happen under such circumstances. 
>  
>
No it is safe to use two workbooks in 2 threads.

Thread 1 -------------------- * Workbook
Workbook 1 ---------------- 1 Thread

>3. Continue here only if u agree to the above. The use of ThreadLocal does NOT 
>increase or decrease the level of thread safety defined above.  
>  
>
Okay, lets say I agree.  Lets say it doesn't.  I still seriously dislike 
this method of workbook context.  

>4. The problem with a static workbook refernce is that when, in a servelet 
>engine, two users and hence two threads are using different workbooks, you 
>still have the same static reference to ONE book, and u get errors. A 
>threadlocal is exactly what is designed to prevent this from happening .. each 
>thread gets its own copy of a variable. And if u see the code, it works only if 
>HSSFWorkbook is thread unsafe (ie, we assume a linear call stack on one thread 
>from a high level call down to a low level call). If and when we make 
>multithreaded use of one workbook possible, then this concept with a 
>threadlocal will NOT work. 
>  
>
My issue is with static workbook references entirely.  I removed 
threadlocal to solve an immediate problem (which
I thought was you're attempt to provide thread saftety as defined that 
many threads being able to access one workbook), I
intended to go back and revisit this entirely.

>5. A threadlocal  in java really has nothing to do with how the system 
>implements threads (unlike for eg, thread local storage in C) Its basically a 
>hashtable, keyed on thread id. So in any system that can give u a thread id, 
>irrespective of how, or even if, it supports threading, it'll work. So in that 
>sense, its really fine.
>  
>
You're wrong here.  

>6. Multi threaded test cases are always difficult, so it'll probably take me a 
>day to come up with one. I have no problems obviously if you can come up with 
>an alternative implementation.. however, passing cell references to the ptgs 
>makes for a class dependency diagram that i would hate. 
>  
>
Thats preferrable to the workbook context being stored as a "current 
book" and the such.  Possibilities

1. Pass the workbook reference
2. Workbook manage its context via "ThreadLocal" as you had it
3. Implement a workbook context manager.

org.apache.poi.hssf.model.context   

ContextManagerInterface
getContext()

ContextManager
ContextManager(HSSFWorkbook)
ContextManager(HSSFWorkbook, HSSFSheet)
ContextManager(HSSFWorkbook, HSSFSheet, HSSFRow)
ContextManager(HSSFWorkbook, HSSFSheet, HSSFRow, HSSFCell)
getContext();

ContextInterface
public final static byte CONTEXT_TYPE_WORKBOOK=0
public final static byte CONTEXT_TYPE_SHEET=1
public final static byte CONTEXT_TYPE_ROW=2
public final static byte CONTEXT_TYPE_CELL=3
getContextType()
getWorkbook()            
getSheet()
getRow()
getCell()

RequiresContextInterface //later we may need specifics like workbook, 
sheet, row, cell....but not yet
setContext(Context);

Anyhow the above is just for example purposes, reality may end up being 
different.

The context should be passed to the constructed item (don't overuse the 
singleton pattern).

This means that the sheet-reference-aware PTGs would be dependant upon 
the context (which is logical).  They'd still be dependant upon the 
workbook.  They'd not need any tricks (ThreadLocal, static variables, 
etc).  And you wouldn't be passing the workbook around.

If this is agreeable, we can convert usermodel.* to work the same way 
internally (currently HSSFRow refers to HSSFSheet & Workbook etc etc). 
 Makes them constructors get smaller and neater too.

Tie that one on and tell me what ya think ;-)

Thanks,

-Andy

>Thoughts?
>-
>Avik
>
>  
>  
>



--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to