>From Michael Blow <mb...@apache.org>:
Michael Blow has uploaded this change for review. (
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20376?usp=email )
Change subject: [NO ISSUE][HYR][STO] FileMapManager performance improvements
......................................................................
[NO ISSUE][HYR][STO] FileMapManager performance improvements
Ext-ref: MB-68501
Change-Id: I6f3a79b3b3e42abd39bc50b149b943a62349c1f8
---
M
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml
M
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
M
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java
A
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/SynchronizedFileMapManager.java
5 files changed, 166 insertions(+), 30 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/76/20376/1
diff --git
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
index c388ba2..73e6429 100644
---
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
+++
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
@@ -42,6 +42,7 @@
import org.apache.hyracks.storage.common.file.BufferedFileHandle;
import org.apache.hyracks.storage.common.file.FileMapManager;
import org.apache.hyracks.storage.common.file.IFileMapManager;
+import org.apache.hyracks.storage.common.file.SynchronizedFileMapManager;
import org.apache.hyracks.util.JSONUtil;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -68,7 +69,7 @@
public VirtualBufferCache(ICacheMemoryAllocator allocator, int pageSize,
int pageBudget) {
this.allocator = allocator;
- this.fileMapManager = new FileMapManager();
+ this.fileMapManager = new SynchronizedFileMapManager(new
FileMapManager());
this.pageSize = pageSize;
if (pageBudget == 0) {
throw new IllegalArgumentException("Page Budget Cannot be 0");
@@ -122,20 +123,13 @@
@Override
public int createFile(FileReference fileRef) throws HyracksDataException {
- synchronized (fileMapManager) {
- return fileMapManager.registerFile(fileRef);
- }
+ return fileMapManager.registerFile(fileRef);
}
@Override
public int openFile(FileReference fileRef) throws HyracksDataException {
try {
- synchronized (fileMapManager) {
- if (fileMapManager.isMapped(fileRef)) {
- return fileMapManager.lookupFileId(fileRef);
- }
- return fileMapManager.registerFile(fileRef);
- }
+ return fileMapManager.registerFileIfAbsent(fileRef);
} finally {
logStats();
}
@@ -159,17 +153,17 @@
@Override
public void deleteFile(FileReference fileRef) throws HyracksDataException {
- synchronized (fileMapManager) {
- int fileId = fileMapManager.lookupFileId(fileRef);
- deleteFile(fileId);
- }
+ int fileId = fileMapManager.unregisterFile(fileRef);
+ reclaimPagesFromDeletedFile(fileId);
}
@Override
public void deleteFile(int fileId) throws HyracksDataException {
- synchronized (fileMapManager) {
- fileMapManager.unregisterFile(fileId);
- }
+ fileMapManager.unregisterFile(fileId);
+ reclaimPagesFromDeletedFile(fileId);
+ }
+
+ private void reclaimPagesFromDeletedFile(int fileId) {
int reclaimedPages = 0;
for (int i = 0; i < buckets.length; i++) {
final CacheBucket bucket = buckets[i];
@@ -243,10 +237,7 @@
}
if (!newPage) {
int fileId = BufferedFileHandle.getFileId(dpid);
- FileReference fileRef;
- synchronized (fileMapManager) {
- fileRef = fileMapManager.lookupFileName(fileId);
- }
+ FileReference fileRef = fileMapManager.lookupFileName(fileId);
throw
HyracksDataException.create(ErrorCode.PAGE_DOES_NOT_EXIST_IN_FILE,
BufferedFileHandle.getPageId(dpid), fileRef);
}
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml
b/hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml
index 33ec4d6..d84ef1e 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml
@@ -74,5 +74,9 @@
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
+ <dependency>
+ <groupId>it.unimi.dsi</groupId>
+ <artifactId>fastutil-core</artifactId>
+ </dependency>
</dependencies>
</project>
\ No newline at end of file
diff --git
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
index 0f2703b..5fa1e9c 100644
---
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
+++
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
@@ -18,23 +18,35 @@
*/
package org.apache.hyracks.storage.common.file;
+import java.io.Serial;
import java.util.Date;
-import java.util.HashMap;
-import java.util.Map;
import org.apache.hyracks.api.exceptions.ErrorCode;
import org.apache.hyracks.api.exceptions.HyracksDataException;
import org.apache.hyracks.api.io.FileReference;
import org.apache.hyracks.util.annotations.NotThreadSafe;
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
+
@NotThreadSafe
public class FileMapManager implements IFileMapManager {
+ @Serial
private static final long serialVersionUID = 1L;
+ private static final int NOT_FOUND = -1;
- private Map<Integer, FileReference> id2nameMap = new HashMap<>();
- private Map<FileReference, Integer> name2IdMap = new HashMap<>();
+ private final Int2ObjectMap<FileReference> id2nameMap;
+ private final Object2IntMap<FileReference> name2IdMap;
private int idCounter = 0;
+ public FileMapManager() {
+ id2nameMap = new Int2ObjectOpenHashMap<>();
+ name2IdMap = new Object2IntOpenHashMap<>();
+ name2IdMap.defaultReturnValue(NOT_FOUND);
+ }
+
@Override
public FileReference lookupFileName(int fileId) throws
HyracksDataException {
FileReference fRef = id2nameMap.get(fileId);
@@ -46,8 +58,8 @@
@Override
public int lookupFileId(FileReference fileRef) throws HyracksDataException
{
- Integer fileId = name2IdMap.get(fileRef);
- if (fileId == null) {
+ int fileId = name2IdMap.getInt(fileRef);
+ if (fileId == NOT_FOUND) {
throw
HyracksDataException.create(ErrorCode.NO_MAPPING_FOR_FILENAME, fileRef);
}
return fileId;
@@ -69,24 +81,48 @@
if (fileRef == null) {
throw
HyracksDataException.create(ErrorCode.NO_MAPPING_FOR_FILE_ID, fileId);
}
- name2IdMap.remove(fileRef);
+ name2IdMap.removeInt(fileRef);
fileRef.unregister();
return fileRef;
}
@Override
+ public int unregisterFile(FileReference fileRef) throws
HyracksDataException {
+ int fileId = name2IdMap.removeInt(fileRef);
+ if (fileId == NOT_FOUND) {
+ throw
HyracksDataException.create(ErrorCode.NO_MAPPING_FOR_FILENAME, fileRef);
+ }
+ id2nameMap.remove(fileId);
+ fileRef.unregister();
+ return fileId;
+ }
+
+ @Override
public int registerFile(FileReference fileRef) throws HyracksDataException
{
- Integer existingKey = name2IdMap.get(fileRef);
- if (existingKey != null) {
+ int existingKey = name2IdMap.getInt(fileRef);
+ if (existingKey != NOT_FOUND) {
FileReference prevFile = id2nameMap.get(existingKey);
throw HyracksDataException.create(ErrorCode.FILE_ALREADY_MAPPED,
fileRef, prevFile,
new Date(prevFile.registrationTime()).toString());
}
+ return registerFileSafe(fileRef);
+ }
+
+ private int registerFileSafe(FileReference fileRef) {
int fileId = idCounter++;
+ if (idCounter == NOT_FOUND) {
+ idCounter++;
+ }
fileRef.register();
id2nameMap.put(fileId, fileRef);
name2IdMap.put(fileRef, fileId);
return fileId;
}
+ @Override
+ public int registerFileIfAbsent(FileReference fileRef) {
+ int fileId = name2IdMap.getInt(fileRef);
+ return fileId != NOT_FOUND ? fileId : registerFileSafe(fileRef);
+ }
+
}
diff --git
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java
index 9a633b3..2a098d2 100644
---
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java
+++
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java
@@ -49,4 +49,23 @@
*/
FileReference unregisterFile(int fileId) throws HyracksDataException;
+ /**
+ * Unregister a file mapping
+ *
+ * @param fileRef
+ * - The file reference whose mapping is to be unregistered.
+ * @throws HyracksDataException
+ * - If the file reference is not mapped currently in this
manager.
+ * @return the file id
+ */
+ int unregisterFile(FileReference fileRef) throws HyracksDataException;
+
+ /**
+ * Register a new file name if it is not already registered.
+ *
+ * @param fileRef
+ * - file reference to register or lookup
+ * @return the file id
+ */
+ int registerFileIfAbsent(FileReference fileRef);
}
diff --git
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/SynchronizedFileMapManager.java
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/SynchronizedFileMapManager.java
new file mode 100644
index 0000000..3d42b2a
--- /dev/null
+++
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/SynchronizedFileMapManager.java
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.hyracks.storage.common.file;
+
+import org.apache.hyracks.api.exceptions.HyracksDataException;
+import org.apache.hyracks.api.io.FileReference;
+
+public class SynchronizedFileMapManager implements IFileMapManager {
+ private final IFileMapManager delegate;
+
+ public SynchronizedFileMapManager(IFileMapManager fileMapManager) {
+ this.delegate = fileMapManager;
+ }
+
+ @Override
+ public int registerFile(FileReference fileRef) throws HyracksDataException
{
+ synchronized (delegate) {
+ return delegate.registerFile(fileRef);
+ }
+ }
+
+ @Override
+ public int registerFileIfAbsent(FileReference fileRef) {
+ synchronized (delegate) {
+ return delegate.registerFileIfAbsent(fileRef);
+ }
+ }
+
+ @Override
+ public FileReference unregisterFile(int fileId) throws
HyracksDataException {
+ synchronized (delegate) {
+ return delegate.unregisterFile(fileId);
+ }
+ }
+
+ @Override
+ public int unregisterFile(FileReference fileRef) throws
HyracksDataException {
+ synchronized (delegate) {
+ return delegate.unregisterFile(fileRef);
+ }
+ }
+
+ @Override
+ public boolean isMapped(int fileId) {
+ synchronized (delegate) {
+ return delegate.isMapped(fileId);
+ }
+ }
+
+ @Override
+ public boolean isMapped(FileReference fileRef) {
+ synchronized (delegate) {
+ return delegate.isMapped(fileRef);
+ }
+ }
+
+ @Override
+ public int lookupFileId(FileReference fileRef) throws HyracksDataException
{
+ synchronized (delegate) {
+ return delegate.lookupFileId(fileRef);
+ }
+ }
+
+ @Override
+ public FileReference lookupFileName(int fileId) throws
HyracksDataException {
+ synchronized (delegate) {
+ return delegate.lookupFileName(fileId);
+ }
+ }
+}
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20376?usp=email
To unsubscribe, or for help writing mail filters, visit
https://asterix-gerrit.ics.uci.edu/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: asterixdb
Gerrit-Branch: trinity
Gerrit-Change-Id: I6f3a79b3b3e42abd39bc50b149b943a62349c1f8
Gerrit-Change-Number: 20376
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Blow <mb...@apache.org>
