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