Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaGrammarInformationProvider performance issue #11

Open
BenjaminKlatt opened this issue Dec 30, 2013 · 2 comments
Open

JavaGrammarInformationProvider performance issue #11

BenjaminKlatt opened this issue Dec 30, 2013 · 2 comments

Comments

@BenjaminKlatt
Copy link
Contributor

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
Copy link
Member

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

@BenjaminKlatt
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants