DevBoost / JaMoPP

JaMoPP can parse Java source and byte code into EMF-based models and vice versa. It preserves source formatting and can be used for code analysis and refactoring.
17 stars 18 forks source link

JavaGrammarInformationProvider performance issue #11

Open BenjaminKlatt opened 10 years ago

BenjaminKlatt commented 10 years ago

The class JavaGrammarInformationProvider contains two methods ("getSyntaxElementID" and "getSyntaxElementByID") called quite often during parsing. jamopp-performance-method-count

They use reflection to always look up an id or object in the same set of static and final fields within the JavaGrammarInformationProvider.

This can be considerably reduced by using a statically initialized cache for this limited set of values.

The JavaGrammarInformationProvider is an EMFText generated class located in /org.emftext.language.java.resource.java/src-gen/org/emftext/language/java/resource/java/grammar/JavaGrammarInformationProvider.java I did not figure out yet how to commit the requried change through a pull request. So please find below the required code changes for a cache using Java native LinkedHashMaps. An alternative would be a cache e.g. based on Google Guava, but this would require an additional dependency for the o.e.l.java.resource.java plugin.

Original Code

    public static String getSyntaxElementID(org.emftext.language.java.resource.java.grammar.JavaSyntaxElement syntaxElement) {
        if (syntaxElement == null) {
            // null indicates EOF
            return "<EOF>";
        }
        for (java.lang.reflect.Field field : org.emftext.language.java.resource.java.grammar.JavaGrammarInformationProvider.class.getFields()) {
            Object fieldValue;
            try {
                fieldValue = field.get(null);
                if (fieldValue == syntaxElement) {
                    String id = field.getName();
                    return id;
                }
            } catch (Exception e) { }
        }
        return null;
    }

    public static org.emftext.language.java.resource.java.grammar.JavaSyntaxElement getSyntaxElementByID(String syntaxElementID) {
        try {
            return (org.emftext.language.java.resource.java.grammar.JavaSyntaxElement) org.emftext.language.java.resource.java.grammar.JavaGrammarInformationProvider.class.getField(syntaxElementID).get(null);
        } catch (Exception e) {
            return null;
        }
    }

Modified Version using local caches

    /** A statically pre-loaded cache to speed up the syntax element id look up. */
    private static LinkedHashMap<JavaSyntaxElement, String> syntaxElementIDCache = new LinkedHashMap<JavaSyntaxElement, String>();

    /** A statically pre-loaded cache to speed up the syntax element look up. */
    private static LinkedHashMap<String, JavaSyntaxElement> syntaxElementCache = new LinkedHashMap<String, JavaSyntaxElement>();

    /** Preload the caches */
    static {
        syntaxElementIDCache.put(null, "<EOF>");
        for (java.lang.reflect.Field field : org.emftext.language.java.resource.java.grammar.JavaGrammarInformationProvider.class.getFields()) {
            Object fieldValue = field.get(null);
            syntaxElementIDCache.put((JavaSyntaxElement) fieldValue, field.getName());
            syntaxElementCache.put(field.getName(), (JavaSyntaxElement) fieldValue);
        }
    }

    public static String getSyntaxElementID(JavaSyntaxElement syntaxElement) {
        return syntaxElementIDCache.get(syntaxElement);
    }

    public static JavaSyntaxElement getSyntaxElementByID(String syntaxElementID) {
        return syntaxElementCache.get(syntaxElementID);
    }
mirkoseifert commented 10 years ago

Excellent. I'll apply the proposed changes to the respective EMFText code generators after my vacation.

BenjaminKlatt commented 10 years ago

I managed to make a fork and pull request for the change. However, referencing this original issue (#11) created an additional one (#12). Sorry for messing up the issue list.