myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam Winer (JIRA)" <...@myfaces.apache.org>
Subject [jira] Commented: (TRINIDAD-773) Inefficient way to create faces ben in FacesBeanFactory
Date Sat, 24 Nov 2007 17:21:43 GMT

    [ https://issues.apache.org/jira/browse/TRINIDAD-773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12545187
] 

Adam Winer commented on TRINIDAD-773:
-------------------------------------

At a quick glance, this checkin has a big thread-safety problem:  FacesBeanFactory can be
called from multiple threads, but you're using a non-threadsafe data structure.

Switch that HashMap to a ConcurrentHashMap and it'll be safe.

> Inefficient way to create faces ben in FacesBeanFactory
> -------------------------------------------------------
>
>                 Key: TRINIDAD-773
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-773
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>            Reporter: Stevan Malesevic
>            Assignee: Jeanne Waldman
>             Fix For: 1.0.5-core
>
>
> It seems that the way FacesBeanFactory::createFacesBean creates faces bean is were inefficient.
The problem is that for the case where we have deep subclass structure and root class defines
the bean we will make a numerouse calls to createFacesBean before we find out the type for
the bean. This will burn the CPU and use memory to create all the keys. One possible optimization
might be to create a map between ownerClass|rendererType and calss for the bean at the top
level where the createFacesBean is called. So, next call will find it right away. I played
with the dirty prototype for this and I was able to see memory improvement of about 140K per
request (I have not measure CPU improvement).
> The prototype I had looked like (were dirty but it works):
> /*
>  *  Licensed to the Apache Software Foundation (ASF) under one
>  *  or more contributor license agreements.  See the NOTICE file
>  *  distributed with this work for additional information
>  *  regarding copyright ownership.  The ASF licenses this file
>  *  to you under the Apache License, Version 2.0 (the
>  *  "License"); you may not use this file except in compliance
>  *  with the License.  You may obtain a copy of the License at
>  * 
>  *  http://www.apache.org/licenses/LICENSE-2.0
>  * 
>  *  Unless required by applicable law or agreed to in writing,
>  *  software distributed under the License is distributed on an
>  *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
>  *  KIND, either express or implied.  See the License for the
>  *  specific language governing permissions and limitations
>  *  under the License.
>  */
> package org.apache.myfaces.trinidad.bean;
> import java.io.InputStream;
> import java.io.IOException;
> import java.net.URL;
> import java.util.ArrayList;
> import java.util.Collections;
> import java.util.Enumeration;
> import java.util.HashMap;
> import java.util.List;
> import java.util.Map;
> import java.util.Properties;
> import org.apache.myfaces.trinidad.logging.TrinidadLogger;
> /**
>  * Base interface for FacesBean storage.
>  *
>  */
> public class FacesBeanFactory
> {
>   /**
>    * Create a FacesBean for a component class.
>    */
>   // TODO change from ownerClass to componentFamily?
>   static public FacesBean createFacesBean(
>     Class<?> ownerClass,
>     String   rendererType)
>   {
>     if (ownerClass == null)
>       return null;
>     String className = ownerClass.getName();
>     FacesBean bean = createFacesBean(className, rendererType);
>     if (bean == null && rendererType != null)
>     {
>       bean = createFacesBean(className, null);
>       
>       if(bean != null)
>       {
>         String typeKey = (rendererType != null)
>                             ? new StringBuilder(className).append("|").append(rendererType).toString()
>                             : className;
>         _TYPES_CLASS.put(typeKey, bean.getClass());
>       }
>     }
>     
>     if (bean == null)
>     {
>       bean = createFacesBean(ownerClass.getSuperclass(), rendererType);
>       
>       if(bean != null)
>       {
>         String typeKey = (rendererType != null)
>                             ? new StringBuilder(className).append("|").append(rendererType).toString()
>                             : className;
>         _TYPES_CLASS.put(typeKey, bean.getClass());
>       }
>     }
>     return bean;
>   }
>   static public FacesBean createFacesBean(
>     String beanType,
>     String rendererType)
>   {
>     String typeKey = (rendererType != null)
>                         ? new StringBuilder(beanType).append("|").append(rendererType).toString()
>                         : beanType;
>     
>     Class<?> type = _TYPES_CLASS.get(typeKey);
>       
>     if(type == null)
>     {    
>       String className = (String) _TYPES_MAP.get(typeKey);
>       if (className == null)
>         return null;
>       
>       try
>       {
>         type = _getClassLoader().loadClass(className);
>         _TYPES_CLASS.put(typeKey, type);
>       }
>       catch (ClassNotFoundException cnfe)
>       {
>         _LOG.severe("CANNOT_FIND_FACESBEAN", className);
>         _LOG.severe(cnfe);
>       }
>     }
>   
>     try
>     {
>       return (FacesBean) type.newInstance();
>     }
>     catch (IllegalAccessException iae)
>     {
>       _LOG.severe("CANNOT_CREATE_FACESBEAN_INSTANCE", type.getName());
>       _LOG.severe(iae);
>     }
>     catch (InstantiationException ie)
>     {
>       _LOG.severe("CANNOT_CREATE_FACESBEAN_INSTANCE", type.getName());
>       _LOG.severe(ie);
>     }
>     return null;
>   }
>   static private void _initializeBeanTypes()
>   {
>     _TYPES_MAP = new HashMap<Object, Object>();
>     List<URL> list = new ArrayList<URL>();
>     try
>     {
>       Enumeration<URL> en = _getClassLoader().getResources(
>                                 "META-INF/faces-bean.properties");
>       while (en.hasMoreElements())
>       {
>         list.add(en.nextElement());
>       }
>       Collections.reverse(list);
>     }
>     catch (IOException ioe)
>     {
>       _LOG.severe(ioe);
>       return;
>     }
>     if (list.isEmpty())
>     {
>       if (_LOG.isInfo())
>         _LOG.info("NO_FACES_BEAN_PROPERTIES_FILES_LOCATED");
>     }
>     for(URL url : list)
>     {
>       _initializeBeanTypes(url);
>     }
>   }
>   static private void _initializeBeanTypes(URL url)
>   {
>     try
>     {
>       Properties properties = new Properties();
>       InputStream is = url.openStream();
>       try
>       {
>         properties.load(is);
>         if (_LOG.isFine())
>           _LOG.fine("Loading bean factory info from " + url);
>         
>         _TYPES_MAP.putAll(properties);
>       }
>       finally
>       {
>         is.close();
>       }
>     }
>     catch (IOException ioe)
>     {
>       _LOG.severe("CANNOT_LOAD_URL", url);
>       _LOG.severe(ioe);
>     }
>   }
>   static private ClassLoader _getClassLoader()
>   {
>     ClassLoader loader = Thread.currentThread().getContextClassLoader();
>     if (loader == null)
>       loader = FacesBeanFactory.class.getClassLoader();
>     return loader;
>   }
>   static private final TrinidadLogger _LOG = TrinidadLogger.createTrinidadLogger(FacesBeanFactory.class);
>   static private       Map<Object, Object> _TYPES_MAP;
>   static private       Map<String, Class<?>> _TYPES_CLASS = new HashMap<String,
Class<?>>();
>   
>   static
>   {
>     _initializeBeanTypes();
>   }
> }

-- 
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