Skip to content

Issue #430: Resolving type system imports through sp is slows things down too much #433

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

Draft
wants to merge 7 commits into
base: release/3.6.x
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

import org.apache.uima.UIMAFramework;
Expand All @@ -44,12 +45,14 @@
import aQute.bnd.osgi.Analyzer;
import aQute.bnd.osgi.Resource;
import aQute.bnd.service.AnalyzerPlugin;
import aQute.bnd.service.Plugin;
import aQute.lib.converter.Converter;
import aQute.service.reporter.Reporter;

@BndPlugin(name = "UIMA")
@BndPlugin(name = "UIMA", parameters = UimaBndPlugin.Configuration.class)
public class UimaBndPlugin
implements AnalyzerPlugin
implements AnalyzerPlugin, Plugin
{

private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private static final Pattern XML_FILE = Pattern.compile(".*\\.xml");
Expand All @@ -58,6 +61,21 @@ public class UimaBndPlugin

private final static XMLParser PARSER = UIMAFramework.getXMLParser();

private Reporter reporter;
private UimaBndPlugin.Configuration configuration;

@Override
public void setProperties(Map<String, String> aMap) throws Exception
{
configuration = Converter.cnv(UimaBndPlugin.Configuration.class, aMap);
}

@Override
public void setReporter(Reporter aReporter)
{
reporter = aReporter;
}

@Override
public boolean analyzeJar(Analyzer analyzer) throws Exception
{
Expand All @@ -69,37 +87,37 @@ public boolean analyzeJar(Analyzer analyzer) throws Exception
for (var entry : resources.entrySet()) {
var path = entry.getKey();
var resource = entry.getValue();

try {
if (XML_FILE.matcher(path).matches()) {
importsProcessed += analyzeXmlFile(analyzer, path, resource);
importsProcessed += analyzeXmlFile(analyzer, path, resource, 0);
}
}
catch (Exception e) {
analyzer.error("Unexpected exception in processing resource (%s): %s", path, e);
}
}
}

LOG.info("UIMA bnd plugin processed {} imports", importsProcessed);

return false;
}

private int analyzeXmlFile(Analyzer analyzer, String path, Resource resource) throws Exception
private int analyzeXmlFile(Analyzer analyzer, String path, Resource resource, int level) throws Exception
{
var desc = readUimaDescriptor(resource);
if (desc == null) {
return 0;
}

LOG.debug("Found {}: {}", desc.getClass().getSimpleName(), path);
LOG.debug("{}Found {}: {}", repeat(" ", level), desc.getClass().getSimpleName(), path);
var imports = getImportsFromDescriptor(desc);

var importsProcessed = 0;
for (var imp : imports) {
if (imp.getName() != null) {
handleImportByName(analyzer, path, imp);
handleImportByName(analyzer, path, imp, level);
importsProcessed++;
continue;
}
Expand All @@ -108,22 +126,21 @@ private int analyzeXmlFile(Analyzer analyzer, String path, Resource resource) th
handleImportByLocation(imp);
continue;
}

LOG.warn(
"Found UIMA type system import without name and location - ignoring, please fix your type system description");
"Found UIMA import without name and location - ignoring, please fix your type system description");
}

return importsProcessed;
}

private void handleImportByLocation(Import imp)
{
LOG.warn(
"Found UIMA type system import by location: {} - ignoring, please only use import-by-name",
LOG.warn("Ignoring UIMA import by location (please only use import-by-name): {}",
imp.getLocation());
}

private void handleImportByName(Analyzer analyzer, String path, Import imp)
private void handleImportByName(Analyzer analyzer, String path, Import imp, int level) throws Exception
{
var tsdPackage = imp.getName();
int lastSeparatorPosition = tsdPackage.lastIndexOf('.');
Expand All @@ -132,18 +149,30 @@ private void handleImportByName(Analyzer analyzer, String path, Import imp)
tsdPackage = tsdPackage.substring(0, lastSeparatorPosition);
}

LOG.debug("Found UIMA type system import by name: {}", tsdPackage);

var pack = analyzer.getPackageRef(tsdPackage);
if (!QN.matcher(pack.getFQN()).matches()) {
analyzer.warning("Type system import does not seem to refer to a package (%s): %s",
path, pack);
analyzer.warning("Import does not seem to refer to a package (%s): %s", path, pack);
}

if (!analyzer.getReferred().containsKey(pack)) {
var alreadyKnownImport = analyzer.getReferred().containsKey(pack);
if (!alreadyKnownImport) {
var attrs = new Attrs();
analyzer.getReferred().put(pack, attrs);
}

LOG.debug("{}Found UIMA import by name: {} {}", repeat(" ", level), tsdPackage, alreadyKnownImport ? "" : "(new)");

var importedResourcePath = imp.getName().replace('.', '/') + ".xml";
var importedResource = analyzer.findResource(importedResourcePath);
if (importedResource == null) {
analyzer.warning("Imported resource not found on classpath: {}", importedResourcePath);
return;
}

if (configuration.transitive(false)) {
LOG.debug("");
analyzeXmlFile(analyzer, importedResourcePath, importedResource, level + 1);
}
}

private List<Import> getImportsFromDescriptor(XMLizable desc)
Expand Down Expand Up @@ -194,4 +223,22 @@ private static <T> List<T> asList(T[] aList)

return Arrays.asList(aList);
}

private static String repeat(String aString, int aCount) {
if (aCount == 0) {
return "";
}

var buf = new StringBuilder();
for (var i = 0; i < aCount; i++) {
buf.append(aString);
}

return buf.toString();
}

public static interface Configuration
{
boolean transitive(boolean aDefault);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public void execute() throws MojoExecutionException {
componentLoader = Util.getClassloader(project, includeScope);

// List of components that is later written to META-INF/org.apache.uima.fit/components.txt
StringBuilder componentsManifest = new StringBuilder();
var componentsManifest = new StringBuilder();

int countGenerated = 0;
for (String file : files) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ private ClassLoaderUtils() {
* <ol>
* <li>The {@link UimaContext} in the {@link UimaContextHolder} of the current thread(if any)</li>
* <li>The current thread-context class loader (if any)</li>
* <li>The class loader through which uimaFIT (i.e. this class) was loaded.</li>
* <li>For backwards compatibility then delegates to {@link #getDefaultClassLoader()}</li>
* <li>The class loader through which UIMA was loaded.</li>
* <li>Finally checks the system classloader</li>
* </ol>
*
* @return a class loader or {@code null} if no suitable class loader could be found.
Expand All @@ -60,7 +60,27 @@ public static ClassLoader findClassLoader() {
return uimaFITClassLoader;
}

return getDefaultClassLoader();
ClassLoader cl;

try {
cl = UIMAFramework.class.getClassLoader();
if (cl != null) {
return cl;
}
} catch (Throwable ex) {
// Fall-through
}

try {
cl = ClassLoader.getSystemClassLoader();
if (cl != null) {
return cl;
}
} catch (Throwable ex) {
// Fall-through
}

return null;
}

/**
Expand Down Expand Up @@ -122,36 +142,4 @@ private static ClassLoader getExtensionClassLoader(ResourceManager aResMgr) {

return null;
}

private static ClassLoader getDefaultClassLoader() {
ClassLoader cl;
try {
cl = Thread.currentThread().getContextClassLoader();
if (cl != null) {
return cl;
}
} catch (Throwable ex) {
// Fall-through
}

try {
cl = UIMAFramework.class.getClassLoader();
if (cl != null) {
return cl;
}
} catch (Throwable ex) {
// Fall-through
}

try {
cl = ClassLoader.getSystemClassLoader();
if (cl != null) {
return cl;
}
} catch (Throwable ex) {
// Fall-through
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public interface RelativePathResolver {
* @return the absolute URL at which the file exists, <code>null</code> it none could be found.
* @deprecated Use {@link #resolveRelativePath(String)} instead.
*/
@Deprecated
@Deprecated(since = "3.6.0")
URL resolveRelativePath(URL aUrl);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.List;
import java.util.StringTokenizer;

import org.apache.uima.UIMAFramework;
import org.apache.uima.resource.RelativePathResolver;

/**
Expand All @@ -59,6 +60,8 @@ public RelativePathResolver_impl() {
}

public RelativePathResolver_impl(ClassLoader aClassLoader) {
mClassLoader = aClassLoader;

// initialize data path based on uima.datapath System property; if not
// present fall back on user.dir
String dataPath = null;
Expand Down Expand Up @@ -88,8 +91,6 @@ public RelativePathResolver_impl(ClassLoader aClassLoader) {
mDataPath = null;
mBaseUrls = null;
}

mClassLoader = aClassLoader;
}

@Override
Expand Down Expand Up @@ -376,9 +377,6 @@ public URL resolveRelativePath(URL aUrl) {
return absURL;
}

/**
* @see org.apache.uima.resource.RelativePathResolver#setPathResolverClassLoader(java.lang.ClassLoader)
*/
@Override
public void setPathResolverClassLoader(ClassLoader aClassLoader) {
mClassLoader = aClassLoader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ public ResourceRegistration(Object aResource, ExternalResourceDescription aDescr
* Cache of imported descriptor URLs from which the parsed objects in importCache were created, so
* that these URLs are not re-parsed if the same URL is imported again.
*/
@Deprecated(since = "3.3.0")
private Map<String, Set<String>> importUrlsCache = Collections.synchronizedMap(new HashMap<>());

/**
Expand Down
Loading