[cssparser-developers] Synchronization in CSSOMParser.java

[Stefan M. <ste...@ho...>]

Hello,

I would like to propose an improvement to CSSOMParser.java.

In my use case, the synchronized code block in the constructor causes a major execution bottleneck. My first patch was to minimize the scope of the synchronized block (see below).
This reduced thread contention by a great deal. But as System.getProperty boils down to Hashtable.get, it also contains synchronized code. To further improve the concurrency of this code part, I need to understand it better. What’s the use case behind retrieving the configuration from system properties? Just to add the possibility for configuration from outside? If this is the case, the synchronization can probably be avoided in cases where CSSOMParser is initiated with a valid Parser object.

Best regards,
Stefan


public CSSOMParser(final Parser parser) {

    if (null != parser) {
        parser_ = parser;

        String parserCanonicalName = parser.getClass().getCanonicalName();
        if (!parserCanonicalName.equals(System.getProperty(SYS_PRPOPERTY))) {
            synchronized (LOCK) {
                System.setProperty(SYS_PRPOPERTY, parserCanonicalName);
            }
        }

        return;
    }

    // no parser provided, determine the correct one
    synchronized (LOCK) {
        String currentParser = System.getProperty("org.w3c.css.sac.parser");
        try {
            // use the direct method if we already failed once before
            if (null != LastFailed_ && LastFailed_.equals(currentParser)) {
                parser_ = new SACParserCSS21();
            }
            else {
                if (null == currentParser) {
                    System.setProperty("org.w3c.css.sac.parser", DEFAULT_PARSER);
                    currentParser = DEFAULT_PARSER;
                }
                final ParserFactory factory = new ParserFactory();
                parser_ = factory.makeParser();
            }
        }
        catch (final Exception e) {
            final Logger log = Logger.getLogger("com.steadystate.css");
            log.warning(e.toString());
            log.warning("using the default 'SACParserCSS21' instead");
            log.throwing("CSSOMParser", "consturctor", e);
            LastFailed_ = currentParser;
            parser_ = new SACParserCSS21();
        }
    }
}