myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jeanne Waldman (JIRA)" <...@myfaces.apache.org>
Subject [jira] Issue Comment Edited: (TRINIDAD-1594) FileSystemStyleCache doesn't scale due to over-synchronization
Date Tue, 13 Oct 2009 01:31:31 GMT

    [ https://issues.apache.org/jira/browse/TRINIDAD-1594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12764909#action_12764909
] 

Jeanne Waldman edited comment on TRINIDAD-1594 at 10/12/09 6:31 PM:
--------------------------------------------------------------------

I did a code review of Blake's changes and this looks good. 

An aside, FileSystemStyleCache has 'features' in it that have never been used in Trinidad.
I noticed that the 'source' in the constructor is always null since it is only called from
SkinStyleProvider, and so the code around _resolver, _sourceFile, etc. should just be deleted.
I'll add this to TRINIDAD-1460. 
The skinning check modified code happens in StyleSheetEntry.java (which needs code reviewing
as well).

      was (Author: jeanne.waldman@oracle.com):
    I did a code review of Blake's changes and this looks good. 

An aside, FileSystemStyleCache has 'features' in it that have never been used in Trinidad.
I noticed that the 'source' in the constructor is always null since it is only called from
SkinStyleProvider, and so the code around _resolver, _sourceFile, etc. should just be deleted.
I'll add this to TRINIDAD-1460. 
The skinning check modified code happens in SkinStyleEntry.java (which needs code reviewing
as well).
  
> FileSystemStyleCache doesn't scale due to over-synchronization
> --------------------------------------------------------------
>
>                 Key: TRINIDAD-1594
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1594
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>          Components: Skinning
>    Affects Versions:  1.2.11-core,  1.2.12-core
>         Environment: All
>            Reporter: Blake Sullivan
>         Attachments: JIRA_1594_1_2_12_2.patch
>
>   Original Estimate: 4h
>  Remaining Estimate: 4h
>
> FileSystemStyleCache doesn't scale under load due to the use of Hashtables for _cache
and _entryCache fields.  Since Hashtables are implicitly synchronized and these objects are
shared across the entire application.  Access to these objects is shared across all users.
 In addition, there is synchronization in the remove case when modification checking is enabled
in _getEntry().  The best fix is to replace these with ConcurrentMaps (which also support
an atomic remove to handle the _getEntry() case)
> Other issues:
> _getDerivationKey() uses a Vector as a temporary object for no apparent reason (the implicit
synchronization isn't needed in this case since the Vector never escapes the function)
> The _resolver is lazily initialized but can never change, so it is better to initialize
this field in the constructor so that it can be made final
> _sourceFile and _baseName never change, so they should be final.
> Note that even with these fixes, there are still further synchronization issues with
this class.  However, the biggest scalability issue is that _getEntry() still contains a synchronized
block in order to deal with the case where modification checking is enabled.  There are also
some safe publication problems, but the most important issue in the future would be getting
rid of the synchronization, at least when modification checking is off

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message