I just checked the code of Geodesic3D and I think that there are still
synchronization problems in this class:
- quadruple() should be synchronized or modified
- getVertex() should be synchronized or modified
- isNeighborVertex() should be synchronized or modified
What do you think ?
Nicolas Vervelle wrote:
> Hi Bob,
>
> I think you're right about the problem with the (vertexCount == null)
> test in the constructor,
> but I would suggest an other fix than the one you have implemented
> because the test in the constructor is here to avoid unnecessary calls
> to a synchronized method (which is bad in general for performance)
>
> My suggestion :
> - keep the test in the constructor
> - use a temporary variable in initialize() so that vertexCount is
> assigned only at the end of the method
>
> Nico
>
> Bob Hanson wrote:
>
>> Miguel,
>>
>> What I meant was that the check for null array must be within the
>> synchronized method initialize(), not in the constructor. So the two
>> changes I effected were
>>
>> a) static added for initialize(), as per Nico's excellent observation
>> b) remove check for null in the Geodesic3D constructor:
>>
>>
>> Geodesic3D(Graphics3D g3d) {
>> this.g3d = g3d;
>> initialize();
>> }
>>
>> private static synchronized void initialize() {
>> if(vertexCounts != null)
>> return;
>> vertexCounts = new short[maxLevel];
>> ....
>> }
>>
>> Do you see what the problem was in having the check for null in the
>> constructor itself?
>>
>> Sorry about the confusion with constructors -- a book I have seemed to
>> suggest that threads cannot yield while constructing, but that's
>> obviously a misreading on my part.
>>
>> The problem was definitely in the missing static keyword, which Nico
>> found... and the misplaced check for the start of initialization, which
>> must appear ONLY within the synchronized method itself, not allowing a
>> bypass of that synchronization. That's because if a second thread is
>> allowed to bypass initialize() and not sync there, waiting for the first
>> thread to complete, then it can skip right to code in Isosurface that
>> requires those normals prior to the first thread completing the
>> initialization. Right?
>>
>>
>>
>
>
>
> _______________________________________________
> Jmol-developers mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/jmol-developers
>
>
>
>
>
_______________________________________________
Jmol-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jmol-developers
