Bug 488501 - optimize storage and reading of strings and versions

Change-Id: I4dcbe3fe97a85597812d6c507a38a535a754e77d
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

3 files changed, 337 insertions, 150 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
index 90b6cad05..7426a9335 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
@@ -12,6 +12,8 @@ package org.eclipse.osgi.tests.container;
 
 import static java.util.jar.Attributes.Name.MANIFEST_VERSION;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
 import java.io.*;
@@ -177,6 +179,21 @@ public class TestModuleContainer extends AbstractTest {
 		adaptor.getContainer().refresh(Arrays.asList(adaptor.getContainer().getModule(0)));
 	}
 
+	// disabled @Test
+	public void testLoadPerformance() throws BundleException, IOException {
+		setupModuleDatabase();
+		ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+		resolvedModuleDatabase.store(new DataOutputStream(bytes), true);
+		bytes.close();
+		System.out.println("SIZE: " + bytes.size());
+		long start = System.currentTimeMillis();
+		for (int i = 0; i < 1000; i++) {
+			DummyContainerAdaptor adaptor = createDummyAdaptor();
+			adaptor.getDatabase().load(new DataInputStream(new ByteArrayInputStream(bytes.toByteArray())));
+		}
+		System.out.println("END: " + (System.currentTimeMillis() - start));
+	}
+
 	@Test
 	public void testSimpleResolve() throws BundleException, IOException {
 		DummyContainerAdaptor adaptor = createDummyAdaptor();
@@ -2294,6 +2311,107 @@ public class TestModuleContainer extends AbstractTest {
 	}
 
 	@Test
+	public void testPersistence() throws BundleException, IOException {
+		DummyContainerAdaptor adaptor = createDummyAdaptor();
+		ModuleContainer container = adaptor.getContainer();
+
+		// install the system.bundle
+		installDummyModule("system.bundle.MF", Constants.SYSTEM_BUNDLE_LOCATION, Constants.SYSTEM_BUNDLE_SYMBOLICNAME, null, null, container);
+
+		Map<String, Object> attrs = new HashMap<String, Object>();
+		attrs.put("string", "sValue");
+		attrs.put("string.list1", Arrays.asList("v1", "v2", "v3"));
+		attrs.put("string.list2", Arrays.asList("v4", "v5", "v6"));
+		attrs.put("version", Version.valueOf("1.1"));
+		attrs.put("version.list", Arrays.asList(Version.valueOf("1.0"), Version.valueOf("2.0"), Version.valueOf("3.0")));
+		attrs.put("long", Long.valueOf(12345));
+		attrs.put("long.list", Arrays.asList(Long.valueOf(1), Long.valueOf(2), Long.valueOf(3)));
+		attrs.put("double", Double.valueOf(1.2345));
+		attrs.put("double.list", Arrays.asList(Double.valueOf(1.1), Double.valueOf(1.2), Double.valueOf(1.3)));
+		attrs.put("uri", "some.uri");
+		attrs.put("set", Arrays.asList("s1", "s2", "s3"));
+
+		// provider with all supported types
+		Map<String, String> providerManifest = new HashMap<String, String>();
+		providerManifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		providerManifest.put(Constants.BUNDLE_SYMBOLICNAME, "provider");
+		providerManifest.put(Constants.EXPORT_PACKAGE, "provider; version=1.1; attr1=attr1; attr2=attr2; dir1:=dir1; dir2:=dir2");
+		providerManifest.put(Constants.PROVIDE_CAPABILITY,
+				"provider.cap;"//
+						+ " string=sValue;"//
+						+ " string.list1:List=\"v1,v2,v3\";"//
+						+ " string.list2:List<String>=\"v4,v5,v6\";"//
+						+ " version:Version=1.1;"//
+						+ " version.list:List<Version>=\"1.0,2.0,3.0\";"//
+						+ " long:Long=12345;"//
+						+ " long.list:List<Long>=\"1,2,3\";"//
+						+ " double:Double=1.2345;"//
+						+ " double.list:List<Double>=\"1.1,1.2,1.3\";"//
+						+ " uri:uri=some.uri;" //
+						+ " set:set=\"s1,s2,s3\"");
+		Module providerModule = installDummyModule(providerManifest, "provider", container);
+		Map<String, Object> providerAttrs = providerModule.getCurrentRevision().getCapabilities("provider.cap").get(0).getAttributes();
+		assertEquals("Wrong provider attrs", attrs, providerAttrs);
+
+		Map<String, String> requirerManifest = new HashMap<String, String>();
+		requirerManifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		requirerManifest.put(Constants.BUNDLE_SYMBOLICNAME, "requirer");
+		requirerManifest.put(Constants.IMPORT_PACKAGE, "provider; version=1.1; attr1=attr1; attr2=attr2; dir1:=dir1; dir2:=dir2");
+		requirerManifest.put(Constants.REQUIRE_CAPABILITY,
+				"optional;"//
+						+ " resolution:=optional; " //
+						+ " string=sValue;"//
+						+ " string.list1:List=\"v1,v2,v3\";"//
+						+ " string.list2:List<String>=\"v4,v5,v6\";"//
+						+ " version:Version=1.1;"//
+						+ " version.list:List<Version>=\"1.0,2.0,3.0\";"//
+						+ " long:Long=12345;"//
+						+ " long.list:List<Long>=\"1,2,3\";"//
+						+ " double:Double=1.2345;"//
+						+ " double.list:List<Double>=\"1.1,1.2,1.3\";"//
+						+ " uri:uri=some.uri;" //
+						+ " set:set=\"s1,s2,s3\"," //
+						+ "provider.cap; filter:=\"(string=sValue)\"," //
+						+ "provider.cap; filter:=\"(string.list1=v2)\"," //
+						+ "provider.cap; filter:=\"(string.list2=v5)\"," //
+						+ "provider.cap; filter:=\"(string.list2=v5)\"," //
+						+ "provider.cap; filter:=\"(&(version>=1.1)(version<=1.1.1))\"," //
+						+ "provider.cap; filter:=\"(&(version.list=1)(version.list=2))\"," //
+						+ "provider.cap; filter:=\"(long>=12344)\"," //
+						+ "provider.cap; filter:=\"(long.list=2)\"," //
+						+ "provider.cap; filter:=\"(double>=1.2)\"," //
+						+ "provider.cap; filter:=\"(double.list=1.2)\"," //
+						+ "provider.cap; filter:=\"(uri=some.uri)\"," //
+						+ "provider.cap; filter:=\"(set=s2)\"" //
+						+ "");
+		Module requirerModule = installDummyModule(requirerManifest, "requirer", container);
+		Map<String, Object> requirerAttrs = requirerModule.getCurrentRevision().getRequirements("optional").get(0).getAttributes();
+		assertEquals("Wrong requirer attrs", attrs, requirerAttrs);
+		ResolutionReport report = container.resolve(Collections.singleton(requirerModule), true);
+		assertNull("Error resolving.", report.getResolutionException());
+
+		ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+		DataOutputStream data = new DataOutputStream(bytes);
+		adaptor.getDatabase().store(data, true);
+
+		// reload into a new container
+		adaptor = createDummyAdaptor();
+		container = adaptor.getContainer();
+		adaptor.getDatabase().load(new DataInputStream(new ByteArrayInputStream(bytes.toByteArray())));
+
+		providerModule = container.getModule("provider");
+		providerAttrs = providerModule.getCurrentRevision().getCapabilities("provider.cap").get(0).getAttributes();
+		assertEquals("Wrong provider attrs", attrs, providerAttrs);
+		assertNotNull("No provider found.", providerModule);
+
+		requirerModule = container.getModule("requirer");
+		assertNotNull("No requirer found.", requirerModule);
+		requirerAttrs = requirerModule.getCurrentRevision().getRequirements("optional").get(0).getAttributes();
+		assertEquals("Wrong requirer attrs", attrs, requirerAttrs);
+
+	}
+
+	@Test
 	public void testBug483849() throws BundleException, IOException {
 		DummyContainerAdaptor adaptor = createDummyAdaptor();
 		ModuleContainer container = adaptor.getContainer();
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java
index 2f2cba462..d0f66bc22 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2014 IBM Corporation and others.
+ * Copyright (c) 2012, 2016 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -11,8 +11,6 @@
 package org.eclipse.osgi.container;
 
 import java.io.*;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -929,17 +927,18 @@ public class ModuleDatabase {
 		private static final int VERSION = 2;
 		private static final byte NULL = 0;
 		private static final byte OBJECT = 1;
+		private static final byte INDEX = 2;
 		private static final byte LONG_STRING = 3;
 		private static final String UTF_8 = "UTF-8"; //$NON-NLS-1$
 
 		private static final byte VALUE_STRING = 0;
-		private static final byte VALUE_STRING_ARRAY = 1;
-		private static final byte VAlUE_BOOLEAN = 2;
-		private static final byte VALUE_INTEGER = 3;
+		// REMOVED treated as List<String> - private static final byte VALUE_STRING_ARRAY = 1;
+		// REMOVED never was really supported by the OSGi builder - private static final byte VAlUE_BOOLEAN = 2;
+		// REMOVED never was really supported by the OSGi builder - private static final byte VALUE_INTEGER = 3;
 		private static final byte VALUE_LONG = 4;
 		private static final byte VALUE_DOUBLE = 5;
 		private static final byte VALUE_VERSION = 6;
-		private static final byte VALUE_URI = 7;
+		// REMOVED treated as type String - private static final byte VALUE_URI = 7;
 		private static final byte VALUE_LIST = 8;
 
 		private static int addToWriteTable(Object object, Map<Object, Integer> objectTable) {
@@ -964,19 +963,47 @@ public class ModuleDatabase {
 			out.writeLong(moduleDatabase.getNextId());
 			out.writeInt(moduleDatabase.getInitialModuleStartLevel());
 
-			List<Module> modules = moduleDatabase.getModules();
-			Map<Object, Integer> objectTable = new HashMap<Object, Integer>();
-			// prime the object table with all the maps
+			// prime the object table with all the strings, versions and maps
+			Set<String> allStrings = new HashSet<String>();
+			Set<Version> allVersions = new HashSet<Version>();
 			Set<Map<String, ?>> allMaps = new HashSet<Map<String, ?>>();
+
+			// first gather all the strings, versions and maps from the modules
+			List<Module> modules = moduleDatabase.getModules();
 			for (Module module : modules) {
-				getMaps(module, allMaps);
+				getStringsVersionsAndMaps(module, moduleDatabase, allStrings, allVersions, allMaps);
 			}
+			// outside of the modules the wirings have 'substituted' packages strings
+			Map<ModuleRevision, ModuleWiring> wirings = moduleDatabase.wirings;
+			for (ModuleWiring wiring : wirings.values()) {
+				Collection<String> substituted = wiring.getSubstitutedNames();
+				for (String pkgName : substituted) {
+					allStrings.add(pkgName);
+				}
+			}
+
+			// Now persist all the Strings
+			Map<Object, Integer> objectTable = new HashMap<Object, Integer>();
+			allStrings.remove(null);
+			out.writeInt(allStrings.size());
+			for (String string : allStrings) {
+				writeString(string, out, objectTable);
+				out.writeInt(addToWriteTable(string, objectTable));
+			}
+			// Followed by versions which may reference strings with their qualifier
+			out.writeInt(allVersions.size());
+			for (Version version : allVersions) {
+				writeVersion(version, out, objectTable);
+				out.writeInt(addToWriteTable(version, objectTable));
+			}
+			// Followed by maps which may reference the strings and versions
 			out.writeInt(allMaps.size());
 			for (Map<String, ?> map : allMaps) {
+				writeMap(map, out, objectTable);
 				out.writeInt(addToWriteTable(map, objectTable));
-				writeMap(map, out);
 			}
-			// prime the object table with all modules
+
+			// Followed by modules which reference the strings, versions, and maps
 			out.writeInt(modules.size());
 			for (Module module : modules) {
 				writeModule(module, moduleDatabase, out, objectTable);
@@ -990,8 +1017,7 @@ public class ModuleDatabase {
 				return;
 			}
 
-			Map<ModuleRevision, ModuleWiring> wirings = moduleDatabase.wirings;
-			// prime the object table with all the required wires
+			// prime the object table with all the required wires which reference the modules
 			out.writeInt(wirings.size());
 			for (ModuleWiring wiring : wirings.values()) {
 				List<ModuleWire> requiredWires = wiring.getPersistentRequiredWires();
@@ -1001,7 +1027,7 @@ public class ModuleDatabase {
 				}
 			}
 
-			// now write all the info about each wiring using only indexes
+			// now write all the info about each wiring using only indexes from the objectTable
 			for (ModuleWiring wiring : wirings.values()) {
 				writeWiring(wiring, out, objectTable);
 			}
@@ -1009,21 +1035,65 @@ public class ModuleDatabase {
 			out.flush();
 		}
 
-		private static void getMaps(Module module, Set<Map<String, ?>> allMaps) {
+		private static void getStringsVersionsAndMaps(Module module, ModuleDatabase moduleDatabase, Set<String> allStrings, Set<Version> allVersions, Set<Map<String, ?>> allMaps) {
 			ModuleRevision current = module.getCurrentRevision();
 			if (current == null)
 				return;
 
+			allStrings.add(module.getLocation());
+			allStrings.add(current.getSymbolicName());
+			allStrings.add(current.getVersion().getQualifier());
+			allVersions.add(current.getVersion());
+			EnumSet<Settings> settings = moduleDatabase.moduleSettings.get(module.getId());
+			if (settings != null) {
+				for (Settings setting : settings) {
+					allStrings.add(setting.toString());
+				}
+			}
+
 			List<ModuleCapability> capabilities = current.getModuleCapabilities(null);
 			for (ModuleCapability capability : capabilities) {
-				allMaps.add(capability.getPersistentAttributes());
-				allMaps.add(capability.getDirectives());
+				allStrings.add(capability.getNamespace());
+				addMap(capability.getPersistentAttributes(), allStrings, allVersions, allMaps);
+				addMap(capability.getDirectives(), allStrings, allVersions, allMaps);
 			}
 
 			List<ModuleRequirement> requirements = current.getModuleRequirements(null);
 			for (ModuleRequirement requirement : requirements) {
-				allMaps.add(requirement.getAttributes());
-				allMaps.add(requirement.getDirectives());
+				allStrings.add(requirement.getNamespace());
+				addMap(requirement.getAttributes(), allStrings, allVersions, allMaps);
+				addMap(requirement.getDirectives(), allStrings, allVersions, allMaps);
+			}
+		}
+
+		private static void addMap(Map<String, ?> map, Set<String> allStrings, Set<Version> allVersions, Set<Map<String, ?>> allMaps) {
+			if (!allMaps.add(map)) {
+				// map was already added
+				return;
+			}
+			for (Map.Entry<String, ?> entry : map.entrySet()) {
+				allStrings.add(entry.getKey());
+				Object value = entry.getValue();
+				if (value instanceof String) {
+					allStrings.add((String) value);
+				} else if (value instanceof Version) {
+					allStrings.add(((Version) value).getQualifier());
+					allVersions.add((Version) value);
+				} else if (value instanceof List) {
+					switch (getListType((List<?>) value)) {
+						case VALUE_STRING :
+							for (Object string : (List<?>) value) {
+								allStrings.add((String) string);
+							}
+							break;
+						case VALUE_VERSION :
+							for (Object version : (List<?>) value) {
+								allStrings.add(((Version) version).getQualifier());
+								allVersions.add((Version) version);
+							}
+							break;
+					}
+				}
 			}
 		}
 
@@ -1038,9 +1108,17 @@ public class ModuleDatabase {
 
 			Map<Integer, Object> objectTable = new HashMap<Integer, Object>();
 			if (version >= 2) {
+				int numStrings = in.readInt();
+				for (int i = 0; i < numStrings; i++) {
+					readIndexedString(in, objectTable);
+				}
+				int numVersions = in.readInt();
+				for (int i = 0; i < numVersions; i++) {
+					readIndexedVersion(in, objectTable);
+				}
 				int numMaps = in.readInt();
 				for (int i = 0; i < numMaps; i++) {
-					readMap(in, objectTable);
+					readIndexedMap(in, objectTable);
 				}
 			}
 			int numModules = in.readInt();
@@ -1087,11 +1165,11 @@ public class ModuleDatabase {
 				return;
 			out.writeInt(addToWriteTable(current, objectTable));
 
-			writeString(module.getLocation(), out);
+			writeString(module.getLocation(), out, objectTable);
 			out.writeLong(module.getId());
 
-			writeString(current.getSymbolicName(), out);
-			writeVersion(current.getVersion(), out);
+			writeString(current.getSymbolicName(), out, objectTable);
+			writeVersion(current.getVersion(), out, objectTable);
 			out.writeInt(current.getTypes());
 
 			List<ModuleCapability> capabilities = current.getModuleCapabilities(null);
@@ -1113,7 +1191,7 @@ public class ModuleDatabase {
 			out.writeInt(settings == null ? 0 : settings.size());
 			if (settings != null) {
 				for (Settings setting : settings) {
-					writeString(setting.name(), out);
+					writeString(setting.name(), out, objectTable);
 				}
 			}
 
@@ -1127,10 +1205,10 @@ public class ModuleDatabase {
 		private static void readModule(ModuleDatabase moduleDatabase, DataInputStream in, Map<Integer, Object> objectTable, int version) throws IOException {
 			ModuleRevisionBuilder builder = new ModuleRevisionBuilder();
 			int moduleIndex = in.readInt();
-			String location = readString(in);
+			String location = readString(in, objectTable);
 			long id = in.readLong();
-			builder.setSymbolicName(readString(in));
-			builder.setVersion(readVersion(in));
+			builder.setSymbolicName(readString(in, objectTable));
+			builder.setVersion(readVersion(in, objectTable));
 			builder.setTypes(in.readInt());
 
 			int numCapabilities = in.readInt();
@@ -1153,7 +1231,7 @@ public class ModuleDatabase {
 			if (numSettings > 0) {
 				settings = EnumSet.noneOf(Settings.class);
 				for (int i = 0; i < numSettings; i++) {
-					settings.add(Settings.valueOf(readString(in)));
+					settings.add(Settings.valueOf(readString(in, objectTable)));
 				}
 			}
 
@@ -1258,7 +1336,7 @@ public class ModuleDatabase {
 			Collection<String> substituted = wiring.getSubstitutedNames();
 			out.writeInt(substituted.size());
 			for (String pkgName : substituted) {
-				writeString(pkgName, out);
+				writeString(pkgName, out, objectTable);
 			}
 		}
 
@@ -1294,14 +1372,14 @@ public class ModuleDatabase {
 			int numSubstitutedNames = in.readInt();
 			Collection<String> substituted = new ArrayList<String>(numSubstitutedNames);
 			for (int i = 0; i < numSubstitutedNames; i++) {
-				substituted.add(readString(in));
+				substituted.add(readString(in, objectTable));
 			}
 
 			return new ModuleWiring(revision, capabilities, requirements, providedWires, requiredWires, substituted);
 		}
 
 		private static void writeGenericInfo(String namespace, Map<String, ?> attributes, Map<String, String> directives, DataOutputStream out, Map<Object, Integer> objectTable) throws IOException {
-			writeString(namespace, out);
+			writeString(namespace, out, objectTable);
 
 			Integer attributesIndex = objectTable.get(attributes);
 			Integer directivesIndex = objectTable.get(directives);
@@ -1313,9 +1391,9 @@ public class ModuleDatabase {
 
 		@SuppressWarnings("unchecked")
 		private static void readGenericInfo(boolean isCapability, DataInputStream in, ModuleRevisionBuilder builder, Map<Integer, Object> objectTable, int version) throws IOException {
-			String namespace = readString(in);
-			Map<String, Object> attributes = version >= 2 ? (Map<String, Object>) objectTable.get(in.readInt()) : readMap(in);
-			Map<String, ?> directives = version >= 2 ? (Map<String, ?>) objectTable.get(in.readInt()) : readMap(in);
+			String namespace = readString(in, objectTable);
+			Map<String, Object> attributes = version >= 2 ? (Map<String, Object>) objectTable.get(in.readInt()) : readMap(in, objectTable);
+			Map<String, ?> directives = version >= 2 ? (Map<String, ?>) objectTable.get(in.readInt()) : readMap(in, objectTable);
 			if (attributes == null || directives == null)
 				throw new NullPointerException("Could not find the expected indexes"); //$NON-NLS-1$
 			if (isCapability) {
@@ -1326,7 +1404,7 @@ public class ModuleDatabase {
 
 		}
 
-		private static void writeMap(Map<String, ?> source, DataOutputStream out) throws IOException {
+		private static void writeMap(Map<String, ?> source, DataOutputStream out, Map<Object, Integer> objectTable) throws IOException {
 			if (source == null) {
 				out.writeInt(0);
 			} else {
@@ -1335,19 +1413,10 @@ public class ModuleDatabase {
 				while (iter.hasNext()) {
 					String key = iter.next();
 					Object value = source.get(key);
-					writeString(key, out);
+					writeString(key, out, objectTable);
 					if (value instanceof String) {
 						out.writeByte(VALUE_STRING);
-						writeString((String) value, out);
-					} else if (value instanceof String[]) {
-						out.writeByte(VALUE_STRING_ARRAY);
-						writeStringArray(out, (String[]) value);
-					} else if (value instanceof Boolean) {
-						out.writeByte(VAlUE_BOOLEAN);
-						out.writeBoolean(((Boolean) value).booleanValue());
-					} else if (value instanceof Integer) {
-						out.writeByte(VALUE_INTEGER);
-						out.writeInt(((Integer) value).intValue());
+						writeString((String) value, out, objectTable);
 					} else if (value instanceof Long) {
 						out.writeByte(VALUE_LONG);
 						out.writeLong(((Long) value).longValue());
@@ -1356,40 +1425,41 @@ public class ModuleDatabase {
 						out.writeDouble(((Double) value).doubleValue());
 					} else if (value instanceof Version) {
 						out.writeByte(VALUE_VERSION);
-						writeVersion((Version) value, out);
-					} else if (value instanceof URI) {
-						out.writeByte(VALUE_URI);
-						writeString(value.toString(), out);
+						writeVersion((Version) value, out, objectTable);
 					} else if (value instanceof List) {
 						out.writeByte(VALUE_LIST);
-						writeList(out, (List<?>) value);
+						writeList(out, (List<?>) value, objectTable);
+					} else {
+						// better do our best and write a string
+						// probably should warn here
+						out.writeByte(VALUE_STRING);
+						writeString((String) value, out, objectTable);
 					}
 				}
 			}
 		}
 
-		private static void readMap(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
-			int mapIndex = in.readInt();
-			Map<String, Object> result = readMap(in);
-			addToReadTable(result, mapIndex, objectTable);
+		private static void readIndexedMap(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
+			Map<String, Object> result = readMap(in, objectTable);
+			addToReadTable(result, in.readInt(), objectTable);
 		}
 
-		private static Map<String, Object> readMap(DataInputStream in) throws IOException {
+		private static Map<String, Object> readMap(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
 			int count = in.readInt();
 			Map<String, Object> result;
 			if (count == 0) {
 				result = Collections.emptyMap();
 			} else if (count == 1) {
-				String key = readString(in);
+				String key = readString(in, objectTable);
 				byte type = in.readByte();
-				Object value = readMapValue(in, type);
+				Object value = readMapValue(in, type, objectTable);
 				result = Collections.singletonMap(key, value);
 			} else {
 				result = new HashMap<String, Object>(count);
 				for (int i = 0; i < count; i++) {
-					String key = readString(in);
+					String key = readString(in, objectTable);
 					byte type = in.readByte();
-					Object value = readMapValue(in, type);
+					Object value = readMapValue(in, type, objectTable);
 					result.put(key, value);
 				}
 				result = Collections.unmodifiableMap(result);
@@ -1397,57 +1467,24 @@ public class ModuleDatabase {
 			return result;
 		}
 
-		private static Object readMapValue(DataInputStream in, int type) throws IOException {
+		private static Object readMapValue(DataInputStream in, int type, Map<Integer, Object> objectTable) throws IOException {
 			switch (type) {
 				case VALUE_STRING :
-					return readString(in);
-				case VALUE_STRING_ARRAY :
-					return readStringArray(in);
-				case VAlUE_BOOLEAN :
-					return in.readBoolean() ? Boolean.TRUE : Boolean.FALSE;
-				case VALUE_INTEGER :
-					return new Integer(in.readInt());
+					return readString(in, objectTable);
 				case VALUE_LONG :
 					return new Long(in.readLong());
 				case VALUE_DOUBLE :
 					return new Double(in.readDouble());
 				case VALUE_VERSION :
-					return readVersion(in);
-				case VALUE_URI :
-					try {
-						return new URI(readString(in));
-					} catch (URISyntaxException e) {
-						return null;
-					}
+					return readVersion(in, objectTable);
 				case VALUE_LIST :
-					return readList(in);
+					return readList(in, objectTable);
 				default :
-					throw new IOException("Invalid type: " + type); //$NON-NLS-1$
+					throw new IllegalArgumentException("Invalid type: " + type); //$NON-NLS-1$
 			}
 		}
 
-		private static void writeStringArray(DataOutputStream out, String[] value) throws IOException {
-			if (value == null) {
-				out.writeInt(0);
-			} else {
-				out.writeInt(value.length);
-				for (int i = 0; i < value.length; i++)
-					writeString(value[i], out);
-			}
-
-		}
-
-		private static String[] readStringArray(DataInputStream in) throws IOException {
-			int count = in.readInt();
-			if (count == 0)
-				return null;
-			String[] result = new String[count];
-			for (int i = 0; i < count; i++)
-				result[i] = readString(in);
-			return result;
-		}
-
-		private static void writeList(DataOutputStream out, List<?> list) throws IOException {
+		private static void writeList(DataOutputStream out, List<?> list, Map<Object, Integer> objectTable) throws IOException {
 			if (list.isEmpty()) {
 				out.writeInt(0);
 				return;
@@ -1462,10 +1499,7 @@ public class ModuleDatabase {
 			for (Object value : list) {
 				switch (type) {
 					case VALUE_STRING :
-						writeString((String) value, out);
-						break;
-					case VALUE_INTEGER :
-						out.writeInt(((Integer) value).intValue());
+						writeString((String) value, out, objectTable);
 						break;
 					case VALUE_LONG :
 						out.writeLong(((Long) value).longValue());
@@ -1474,7 +1508,7 @@ public class ModuleDatabase {
 						out.writeDouble(((Double) value).doubleValue());
 						break;
 					case VALUE_VERSION :
-						writeVersion((Version) value, out);
+						writeVersion((Version) value, out, objectTable);
 						break;
 					default :
 						break;
@@ -1488,8 +1522,6 @@ public class ModuleDatabase {
 			Object type = list.get(0);
 			if (type instanceof String)
 				return VALUE_STRING;
-			if (type instanceof Integer)
-				return VALUE_INTEGER;
 			if (type instanceof Long)
 				return VALUE_LONG;
 			if (type instanceof Double)
@@ -1499,68 +1531,94 @@ public class ModuleDatabase {
 			return -2;
 		}
 
-		private static List<?> readList(DataInputStream in) throws IOException {
+		private static List<?> readList(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
 			int size = in.readInt();
 			if (size == 0)
 				return Collections.emptyList();
 			byte listType = in.readByte();
 			if (size == 1) {
-				return Collections.singletonList(readListValue(listType, in));
+				return Collections.singletonList(readListValue(listType, in, objectTable));
 			}
 			List<Object> list = new ArrayList<Object>(size);
 			for (int i = 0; i < size; i++) {
-				list.add(readListValue(listType, in));
+				list.add(readListValue(listType, in, objectTable));
 			}
 			return Collections.unmodifiableList(list);
 		}
 
-		private static Object readListValue(byte listType, DataInputStream in) throws IOException {
+		private static Object readListValue(byte listType, DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
 			switch (listType) {
 				case VALUE_STRING :
-					return readString(in);
-				case VALUE_INTEGER :
-					return new Integer(in.readInt());
+					return readString(in, objectTable);
 				case VALUE_LONG :
 					return new Long(in.readLong());
 				case VALUE_DOUBLE :
 					return new Double(in.readDouble());
 				case VALUE_VERSION :
-					return readVersion(in);
+					return readVersion(in, objectTable);
 				default :
-					throw new IOException("Invalid type: " + listType); //$NON-NLS-1$
+					throw new IllegalArgumentException("Invalid type: " + listType); //$NON-NLS-1$
 			}
 		}
 
-		private static void writeVersion(Version version, DataOutputStream out) throws IOException {
+		private static void writeVersion(Version version, DataOutputStream out, Map<Object, Integer> objectTable) throws IOException {
 			if (version == null || version.equals(Version.emptyVersion)) {
 				out.writeByte(NULL);
 				return;
 			}
+			Integer index = objectTable.get(version);
+			if (index != null) {
+				out.writeByte(INDEX);
+				out.writeInt(index);
+				return;
+			}
 			out.writeByte(OBJECT);
 			out.writeInt(version.getMajor());
 			out.writeInt(version.getMinor());
 			out.writeInt(version.getMicro());
-			writeQualifier(version.getQualifier(), out);
+			writeQualifier(version.getQualifier(), out, objectTable);
 		}
 
-		private static void writeQualifier(String string, DataOutputStream out) throws IOException {
+		private static void writeQualifier(String string, DataOutputStream out, Map<Object, Integer> objectTable) throws IOException {
 			if (string != null && string.length() == 0)
 				string = null;
-			writeString(string, out);
+			writeString(string, out, objectTable);
+		}
+
+		private static Version readIndexedVersion(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
+			Version version = readVersion0(in, objectTable, false);
+			addToReadTable(version, in.readInt(), objectTable);
+			return version;
+		}
+
+		private static Version readVersion(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
+			return readVersion0(in, objectTable, true);
 		}
 
-		private static Version readVersion(DataInputStream in) throws IOException {
-			byte tag = in.readByte();
-			if (tag == NULL)
+		private static Version readVersion0(DataInputStream in, Map<Integer, Object> objectTable, boolean intern) throws IOException {
+			byte type = in.readByte();
+			if (type == INDEX) {
+				int index = in.readInt();
+				return (Version) objectTable.get(index);
+			}
+			if (type == NULL)
 				return Version.emptyVersion;
 			int majorComponent = in.readInt();
 			int minorComponent = in.readInt();
 			int serviceComponent = in.readInt();
-			String qualifierComponent = readString(in);
-			return ObjectPool.intern(new Version(majorComponent, minorComponent, serviceComponent, qualifierComponent));
+			String qualifierComponent = readString(in, objectTable);
+			Version version = new Version(majorComponent, minorComponent, serviceComponent, qualifierComponent);
+			return intern ? ObjectPool.intern(version) : version;
 		}
 
-		private static void writeString(String string, DataOutputStream out) throws IOException {
+		private static void writeString(String string, DataOutputStream out, Map<Object, Integer> objectTable) throws IOException {
+			Integer index = string != null ? objectTable.get(string) : null;
+			if (index != null) {
+				out.writeByte(INDEX);
+				out.writeInt(index);
+				return;
+			}
+
 			if (string == null)
 				out.writeByte(NULL);
 			else {
@@ -1577,21 +1635,36 @@ public class ModuleDatabase {
 			}
 		}
 
-		static private String readString(DataInputStream in) throws IOException {
+		static private String readIndexedString(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
+			String string = readString0(in, objectTable, false);
+			addToReadTable(string, in.readInt(), objectTable);
+			return string;
+		}
+
+		static private String readString(DataInputStream in, Map<Integer, Object> objectTable) throws IOException {
+			return readString0(in, objectTable, true);
+		}
+
+		static private String readString0(DataInputStream in, Map<Integer, Object> objectTable, boolean intern) throws IOException {
 			byte type = in.readByte();
-			if (type == NULL)
+			if (type == INDEX) {
+				int index = in.readInt();
+				return (String) objectTable.get(index);
+			}
+			if (type == NULL) {
 				return null;
-
+			}
+			String string;
 			if (type == LONG_STRING) {
 				int length = in.readInt();
 				byte[] data = new byte[length];
 				in.readFully(data);
-				String string = new String(data, UTF_8);
-
-				return ObjectPool.intern(string);
+				string = new String(data, UTF_8);
+			} else {
+				string = in.readUTF();
 			}
 
-			return ObjectPool.intern(in.readUTF());
+			return intern ? ObjectPool.intern(string) : string;
 		}
 	}
 }
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java
index fe047036e..b3b568aca 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java
@@ -10,8 +10,6 @@
  *******************************************************************************/
 package org.eclipse.osgi.container.builders;
 
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.*;
 import org.eclipse.osgi.container.ModuleRevisionBuilder;
 import org.eclipse.osgi.container.namespaces.*;
@@ -668,26 +666,24 @@ public final class OSGiManifestBuilderFactory {
 	}
 
 	private static Object convertValue(String type, String value) {
-
-		if (ATTR_TYPE_STRING.equalsIgnoreCase(type))
+		if (ATTR_TYPE_STRING.equalsIgnoreCase(type)) {
 			return value;
+		}
 
 		String trimmed = value.trim();
-		if (ATTR_TYPE_DOUBLE.equalsIgnoreCase(type))
+		if (ATTR_TYPE_DOUBLE.equalsIgnoreCase(type)) {
 			return new Double(trimmed);
-		else if (ATTR_TYPE_LONG.equalsIgnoreCase(type))
+		} else if (ATTR_TYPE_LONG.equalsIgnoreCase(type)) {
 			return new Long(trimmed);
-		else if (ATTR_TYPE_URI.equalsIgnoreCase(type))
-			try {
-				return new URI(trimmed);
-			} catch (URISyntaxException e) {
-				throw new RuntimeException(e);
-			}
-		else if (ATTR_TYPE_VERSION.equalsIgnoreCase(type))
+		} else if (ATTR_TYPE_URI.equalsIgnoreCase(type)) {
+			// we no longer actually create URIs here; just use the string
+			return trimmed;
+		} else if (ATTR_TYPE_VERSION.equalsIgnoreCase(type)) {
 			return new Version(trimmed);
-		else if (ATTR_TYPE_SET.equalsIgnoreCase(type))
-			return ManifestElement.getArrayFromList(trimmed, ","); //$NON-NLS-1$
-
+		} else if (ATTR_TYPE_SET.equalsIgnoreCase(type)) {
+			// just use List<String> here so we don't have to deal with String[] in other places
+			return Collections.unmodifiableList(Arrays.asList(ManifestElement.getArrayFromList(trimmed, ","))); //$NON-NLS-1$
+		}
 		// assume list type, anything else will throw an exception
 		Tokenizer listTokenizer = new Tokenizer(type);
 		String listType = listTokenizer.getToken("<"); //$NON-NLS-1$