[vfs][refactoring] Making a proper StorageFactory for PersistentFSRecordsStorage

GitOrigin-RevId: 727d15fe90bc1f54f09dcc0f5fdcad05304ffdaa
2026-09-27 10:03:11 +07:00 · 2024-03-25 23:31:00 +00:00
parent 3e5f57202e
commit c7cf8c153a
5 changed files with 111 additions and 153 deletions
@@ -244,7 +244,7 @@ public final class FSRecordsImpl implements Closeable {
    //bumped main version (60 -> 61) because of PFSRecords header enlargement (HEADER_ERRORS_ACCUMULATED)
    final int mainVFSFormatVersion = 61;
    //@formatter:off (nextMask better be aligned)
-    return nextMask(mainVFSFormatVersion + (PersistentFSRecordsStorageFactory.getRecordsStorageImplementation().ordinal()), /* acceptable range is [0..255] */ 8,
+    return nextMask(mainVFSFormatVersion + (PersistentFSRecordsStorageFactory.recordsStorageImplementation().ordinal()), /* acceptable range is [0..255] */ 8,
           nextMask(!USE_CONTENT_STORAGE_OVER_MMAPPED_FILE,  //former USE_CONTENT_HASHES=true, this is why negation
           nextMask(IOUtil.useNativeByteOrderForByteBuffers(),
           nextMask(PageCacheUtils.LOCK_FREE_PAGE_CACHE_ENABLED && USE_ATTRIBUTES_OVER_NEW_FILE_PAGE_CACHE,//pageSize was changed on old<->new transition
@@ -27,6 +27,7 @@ import com.intellij.util.io.*;
 import com.intellij.util.io.blobstorage.SpaceAllocationStrategy;
 import com.intellij.util.io.blobstorage.SpaceAllocationStrategy.DataLengthPlusFixedPercentStrategy;
 import com.intellij.util.io.blobstorage.StreamlinedBlobStorage;
+import com.intellij.util.io.dev.StorageFactory;
 import com.intellij.util.io.dev.mmapped.MMappedFileStorageFactory;
 import com.intellij.util.io.pagecache.impl.PageContentLockingStrategy;
 import com.intellij.util.io.storage.*;
@@ -86,7 +87,7 @@ public final class PersistentFSLoader {
      })
      .filter(e -> e instanceof IOException)
      .findFirst().orElse(null);
-    
+
    if (mainIoException != null && !mainIoException.getMessage().isEmpty()) {
      for (Throwable exception : exceptions) {
        if (exception != mainIoException) {
@@ -776,15 +777,21 @@ public final class PersistentFSLoader {
  }

  public @NotNull PersistentFSRecordsStorage createRecordsStorage(@NotNull Path recordsFile) throws IOException {
-    PersistentFSRecordsStorage storage = PersistentFSRecordsStorageFactory.createStorage(recordsFile);
-    if (vfsLog != null) {
-      var recordsInterceptors = vfsLog.getConnectionInterceptors().stream()
-        .filter(RecordsInterceptor.class::isInstance)
-        .map(RecordsInterceptor.class::cast)
-        .toList();
-      storage = InterceptorInjection.INSTANCE.injectInRecords(storage, recordsInterceptors);
-    }
-    return storage;
+    StorageFactory<PersistentFSRecordsStorage> recordsStorageFactory = PersistentFSRecordsStorageFactory.recordsStorageImplementation();
+
+    LOG.trace("VFS uses " + recordsStorageFactory + " storage for main file records table");
+    return recordsStorageFactory.wrapStorageSafely(recordsFile, records -> {
+      if (vfsLog != null) {
+        var recordsInterceptors = vfsLog.getConnectionInterceptors().stream()
+          .filter(RecordsInterceptor.class::isInstance)
+          .map(RecordsInterceptor.class::cast)
+          .toList();
+        return InterceptorInjection.INSTANCE.injectInRecords(records, recordsInterceptors);
+      }
+      else {
+        return records;
+      }
+    });
  }

  /** @return common version of all 3 storages, or -1, if their versions are different (i.e. inconsistent) */
@@ -8,7 +8,7 @@ import org.jetbrains.annotations.NotNull;
 import java.io.IOException;

@ApiStatus.Internal
-public interface PersistentFSRecordsStorage extends CleanableStorage {
+public interface PersistentFSRecordsStorage extends CleanableStorage, AutoCloseable {
  int NULL_ID = FSRecords.NULL_FILE_ID;
  int MIN_VALID_ID = NULL_ID + 1;

@@ -118,10 +118,11 @@ public interface PersistentFSRecordsStorage extends CleanableStorage {

  // TODO add a synchronization or requirement to be called on the loading
  @SuppressWarnings("UnusedReturnValue")
-  boolean processAllRecords(@NotNull PersistentFSRecordsStorage.FsRecordProcessor processor) throws IOException;
+  boolean processAllRecords(@NotNull FsRecordProcessor processor) throws IOException;

  void force() throws IOException;

+  @Override
  void close() throws IOException;

  /** Close the storage and remove all its data files */
@@ -1,9 +1,12 @@
-// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.openapi.vfs.newvfs.persistent

-import com.intellij.openapi.diagnostic.Logger
 import com.intellij.util.SystemProperties.getBooleanProperty
-import com.intellij.util.io.*
+import com.intellij.util.io.IOUtil
+import com.intellij.util.io.PageCacheUtils
+import com.intellij.util.io.PagedFileStorageWithRWLockedPageContent
+import com.intellij.util.io.StorageLockContext
+import com.intellij.util.io.dev.StorageFactory
 import com.intellij.util.io.dev.mmapped.MMappedFileStorageFactory
 import com.intellij.util.io.pagecache.impl.PageContentLockingStrategy
 import org.jetbrains.annotations.VisibleForTesting
@@ -13,36 +16,85 @@ import kotlin.io.path.exists


 object PersistentFSRecordsStorageFactory {
-  /**
-   * Fallback option: usually [PersistentFSConnector] deals with storages version changes just fine.
-   * But there are cases where it fails: e.g. with mmapped storages on Win, there file once mapped is hard
-   * to remove (and [PersistentFSConnector] relies on ability to just remove legacy storages files).
-   * In such cases, this option could help get rid of the legacy file early, before it is mapped
-   * MAYBE RC: we probably outgrow that issues:
-   *           a) it's been 2 releases since we moved to memory-mapped for fs-records, i.e. everybody who could have this
-   *           problem -- already has fixed it
-   *           b) i've done a lot for memory-mapped files to work properly on Windows also
-   */
-  @JvmStatic
-  private val FAIL_EARLY_IF_LEGACY_STORAGE_DETECTED: Boolean = getBooleanProperty("vfs.fail-early-if-legacy-storage-detected", false)

-  private val PERSISTENT_FS_STORAGE_CONTEXT_RW = StorageLockContext(true, true, true)
+  enum class RecordsStorageKind : StorageFactory<PersistentFSRecordsStorage> {

-  enum class RecordsStorageKind {
    /** Currently the default impl */
-    OVER_MMAPPED_FILE,
+    OVER_MMAPPED_FILE {
+      /**
+       * Fallback option: usually [PersistentFSConnector] deals with storages version changes just fine.
+       * But there are cases where it fails: e.g. with mmapped storages on Win, there file once mapped is hard
+       * to remove (and [PersistentFSConnector] relies on ability to just remove legacy storages files).
+       * In such cases, this option could help get rid of the legacy file early, before it is mapped
+       * MAYBE RC: we probably outgrow that issues:
+       *           a) it's been 2 releases since we moved to memory-mapped for fs-records, i.e. everybody who could have this
+       *           problem -- already has fixed it
+       *           b) i've done a lot for memory-mapped files to work properly on Windows also
+       */
+      private val FAIL_EARLY_IF_LEGACY_STORAGE_DETECTED: Boolean = getBooleanProperty("vfs.fail-early-if-legacy-storage-detected", false)
+
+      override fun open(storagePath: Path): PersistentFSRecordsLockFreeOverMMappedFile {
+        //TODO RC: this should be replaced with/encapsulated into StorageFactory<PersistentFSStorage>
+        if (FAIL_EARLY_IF_LEGACY_STORAGE_DETECTED) {
+          val legacyLengthFile = storagePath.resolveSibling(storagePath.fileName.toString() + ".len")
+          if (legacyLengthFile.exists()) {
+            //MAYBE RC: actually, here we can _migrate_ legacy file to the new format: move the files to the tmp folder,
+            // create empty new storage, and just copy all records from legacy storage to the new one...
+            throw IOException("Legacy records file detected (${legacyLengthFile} exists): VFS rebuild required")
+          }
+        }
+        return MMappedFileStorageFactory.withDefaults()
+          .pageSize(PersistentFSRecordsLockFreeOverMMappedFile.DEFAULT_MAPPED_CHUNK_SIZE)
+          .wrapStorageSafely<PersistentFSRecordsLockFreeOverMMappedFile, IOException>(storagePath) { storage ->
+            PersistentFSRecordsLockFreeOverMMappedFile(storage)
+          }
+      }
+    },
+
    /** Fallback impl for [OVER_MMAPPED_FILE] if something goes terribly wrong, and we can't fix it quickly */
-    OVER_LOCK_FREE_FILE_CACHE,
+    OVER_LOCK_FREE_FILE_CACHE {
+      private val PERSISTENT_FS_STORAGE_CONTEXT_RW = StorageLockContext(true, true, true)
+
+      override fun open(storagePath: Path): PersistentFSRecordsOverLockFreePagedStorage {
+        val recordLength = PersistentFSRecordsOverLockFreePagedStorage.RECORD_SIZE_IN_BYTES
+        val pageSize = PageCacheUtils.DEFAULT_PAGE_SIZE
+
+        if (!PageCacheUtils.LOCK_FREE_PAGE_CACHE_ENABLED) {
+          throw IOException(
+            "Configuration mismatch: PageCacheUtils.LOCK_FREE_PAGE_CACHE_ENABLED=false " +
+            "=> can't create PersistentFSRecordsOverLockFreePagedStorage if FilePageCacheLockFree is disabled")
+        }
+
+        val recordsArePageAligned = pageSize % recordLength == 0
+        if (!recordsArePageAligned) {
+          throw AssertionError("Bug: record length(=$recordLength) is not aligned with page size(=$pageSize)")
+        }
+
+        val pagedStorage = PagedFileStorageWithRWLockedPageContent(
+          storagePath,
+          PERSISTENT_FS_STORAGE_CONTEXT_RW,
+          pageSize,
+          IOUtil.useNativeByteOrderForByteBuffers(),
+          PageContentLockingStrategy.LOCK_PER_PAGE
+        )
+        return IOUtil.wrapSafely<PersistentFSRecordsOverLockFreePagedStorage, PagedFileStorageWithRWLockedPageContent, IOException>(pagedStorage){
+          PersistentFSRecordsOverLockFreePagedStorage(it)
+        }
+      }
+    },
+
    /** For testing/benchmarking: serves as a reference point. Not a prod-level implementation! */
-    IN_MEMORY
+    IN_MEMORY {
+      override fun open(storagePath: Path) =
+        @Suppress("TestOnlyProblems") PersistentInMemoryFSRecordsStorage(storagePath,  /*max size: */1 shl 24)
+    }
  }

-  private var RECORDS_STORAGE_KIND = RecordsStorageKind.valueOf(
-    System.getProperty("vfs.records-storage.impl", RecordsStorageKind.OVER_MMAPPED_FILE.name))
+  private var RECORDS_STORAGE_KIND = defaultFromSystemProperties()


  @JvmStatic
-  fun getRecordsStorageImplementation(): RecordsStorageKind = RECORDS_STORAGE_KIND
+  fun recordsStorageImplementation(): RecordsStorageKind = RECORDS_STORAGE_KIND

  @VisibleForTesting
  @JvmStatic
@@ -55,88 +107,9 @@ object PersistentFSRecordsStorageFactory {
  @JvmStatic
  @JvmName("resetRecordsStorageImplementation")
  fun resetRecordsStorageImplementation() {
-    RECORDS_STORAGE_KIND = RecordsStorageKind.valueOf(System.getProperty("vfs.records-storage.impl", RecordsStorageKind.OVER_MMAPPED_FILE.name))
+    RECORDS_STORAGE_KIND = defaultFromSystemProperties()
  }

-
-  @JvmStatic
-  @Throws(IOException::class)
-  fun createStorage(file: Path): PersistentFSRecordsStorage {
-    FSRecords.LOG.trace("using $RECORDS_STORAGE_KIND storage for VFS records")
-
-    return when (RECORDS_STORAGE_KIND) {
-      RecordsStorageKind.IN_MEMORY -> @Suppress("TestOnlyProblems") PersistentInMemoryFSRecordsStorage(file,  /*max size: */1 shl 24)
-      RecordsStorageKind.OVER_LOCK_FREE_FILE_CACHE -> createLockFreeStorage(file)
-      RecordsStorageKind.OVER_MMAPPED_FILE -> {
-        //TODO RC: this should be replaced with/encapsulated into StorageFactory<PersistentFSStorage>
-        if (FAIL_EARLY_IF_LEGACY_STORAGE_DETECTED) {
-          val legacyLengthFile = file.resolveSibling(file.fileName.toString() + ".len")
-          if (legacyLengthFile.exists()) {
-            //MAYBE RC: actually, here we can _migrate_ legacy file to the new format: move the files to the tmp folder,
-            // create empty new storage, and just copy all records from legacy storage to the new one...
-            throw IOException("Legacy records file detected (${legacyLengthFile} exists): VFS rebuild required")
-          }
-        }
-        MMappedFileStorageFactory.withDefaults()
-          .pageSize(PersistentFSRecordsLockFreeOverMMappedFile.DEFAULT_MAPPED_CHUNK_SIZE)
-          .wrapStorageSafely<PersistentFSRecordsLockFreeOverMMappedFile, IOException>(file) { storage ->
-            PersistentFSRecordsLockFreeOverMMappedFile(storage)
-          }
-      }
-    }
-  }
-
-  @JvmStatic
-  @VisibleForTesting
-  @Throws(IOException::class)
-  fun openRMappedFile(file: Path,
-                      recordLength: Int): ResizeableMappedFile {
-    val pageSize = PageCacheUtils.DEFAULT_PAGE_SIZE * recordLength / PersistentFSRecordsOverLockFreePagedStorage.RECORD_SIZE_IN_BYTES
-
-    val aligned = pageSize % recordLength == 0
-    if (!aligned) {
-      val message = "Record length(=$recordLength) is not aligned with page size(=$pageSize)"
-      Logger.getInstance(PersistentFSRecordsStorage::class.java).error(message)
-    }
-
-    return ResizeableMappedFile(file, recordLength * 1024,
-                                PERSISTENT_FS_STORAGE_CONTEXT_RW,
-                                pageSize,
-                                aligned,
-                                IOUtil.useNativeByteOrderForByteBuffers())
-  }
-
-  @JvmStatic
-  @VisibleForTesting
-  @Throws(IOException::class)
-  fun createLockFreeStorage(file: Path): PersistentFSRecordsOverLockFreePagedStorage {
-    val recordLength = PersistentFSRecordsOverLockFreePagedStorage.RECORD_SIZE_IN_BYTES
-    val pageSize = PageCacheUtils.DEFAULT_PAGE_SIZE
-
-    if (!PageCacheUtils.LOCK_FREE_PAGE_CACHE_ENABLED) {
-      throw AssertionError(
-        "Bug: PageCacheUtils.LOCK_FREE_PAGE_CACHE_ENABLED=false " +
-        "=> can't create PersistentFSRecordsOverLockFreePagedStorage if FilePageCacheLockFree is disabled")
-    }
-
-    val recordsArePageAligned = pageSize % recordLength == 0
-    if (!recordsArePageAligned) {
-      throw AssertionError("Bug: record length(=$recordLength) is not aligned with page size(=$pageSize)")
-    }
-
-    val storage = PagedFileStorageWithRWLockedPageContent(
-      file,
-      PERSISTENT_FS_STORAGE_CONTEXT_RW,
-      pageSize,
-      IOUtil.useNativeByteOrderForByteBuffers(),
-      PageContentLockingStrategy.LOCK_PER_PAGE
-    )
-    try {
-      return PersistentFSRecordsOverLockFreePagedStorage(storage)
-    }
-    catch (t: Throwable) {
-      storage.close()
-      throw t
-    }
-  }
+  private fun defaultFromSystemProperties() =
+    RecordsStorageKind.valueOf(System.getProperty("vfs.records-storage.impl", RecordsStorageKind.OVER_MMAPPED_FILE.name))
 }
@@ -1,13 +1,10 @@
-// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.openapi.vfs.newvfs.persistent;


+import com.intellij.openapi.vfs.newvfs.persistent.PersistentFSRecordsStorageFactory.RecordsStorageKind;
 import com.intellij.util.io.PageCacheUtils;
-import com.intellij.util.io.PagedFileStorageWithRWLockedPageContent;
-import com.intellij.util.io.StorageLockContext;
-import com.intellij.util.io.pagecache.impl.PageContentLockingStrategy;
 import org.jetbrains.annotations.NotNull;
-import org.junit.After;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -35,14 +32,12 @@ public class PersistentFSRecordsStorageOverLockFreePagedStorageTest
  }

  public static final int MAX_RECORDS_TO_INSERT = 1 << 22;
-  private PagedFileStorageWithRWLockedPageContent pagedStorage;

+  private static final int PAGE_SIZE = PageCacheUtils.DEFAULT_PAGE_SIZE;


  public PersistentFSRecordsStorageOverLockFreePagedStorageTest(final UpdateAPIMethod updateMethodToTest) { super(MAX_RECORDS_TO_INSERT, updateMethodToTest); }

-  private StorageLockContext storageContext;
-
  @BeforeClass
  public static void beforeClass() throws Exception {
    assumeTrue(
@@ -54,43 +49,27 @@ public class PersistentFSRecordsStorageOverLockFreePagedStorageTest
  @NotNull
  @Override
  protected PersistentFSRecordsOverLockFreePagedStorage openStorage(final Path storagePath) throws IOException {
-    final int pageSize;
-    final boolean nativeBytesOrder;
-    try (var file = PersistentFSRecordsStorageFactory.openRMappedFile(storagePath, RECORD_SIZE_IN_BYTES)) {
-      storageContext = file.getStorageLockContext();
-      pageSize = file.getPagedFileStorage().getPageSize();
-      nativeBytesOrder = file.isNativeBytesOrder();
-    }
-    pagedStorage = new PagedFileStorageWithRWLockedPageContent(
-      storagePath,
-      storageContext,
-      pageSize,
-      nativeBytesOrder,
-      PageContentLockingStrategy.LOCK_PER_PAGE
-    );
-    return new PersistentFSRecordsOverLockFreePagedStorage(pagedStorage);
+    return (PersistentFSRecordsOverLockFreePagedStorage)RecordsStorageKind.OVER_LOCK_FREE_FILE_CACHE.open(storagePath);
  }

  @Test
  public void recordAreAlwaysAlignedFullyOnSinglePage() {
-    final int pageSize = pagedStorage.getPageSize();
-    final int enoughRecords = (pageSize / RECORD_SIZE_IN_BYTES) * 16;
+    final int enoughRecords = (PAGE_SIZE / RECORD_SIZE_IN_BYTES) * 16;

    for (int recordId = 0; recordId < enoughRecords; recordId++) {
      final long recordOffsetInFile = storage.recordOffsetInFileUnchecked(recordId);
      final long recordEndOffsetInFile = recordOffsetInFile + RECORD_SIZE_IN_BYTES - 1;
      assertEquals(
        "Record(#" + recordId + ", offset: " + recordOffsetInFile + ") must start and end on a same page",
-        recordOffsetInFile / pageSize,
-        recordEndOffsetInFile / pageSize
+        recordOffsetInFile / PAGE_SIZE,
+        recordEndOffsetInFile / PAGE_SIZE
      );
    }
  }

  @Test
  public void recordOffsetCalculatedByStorageIsConsistentWithPlainCalculation() {
-    final int pageSize = pagedStorage.getPageSize();
-    final int enoughRecords = (pageSize / RECORD_SIZE_IN_BYTES) * 16;
+    final int enoughRecords = (PAGE_SIZE / RECORD_SIZE_IN_BYTES) * 16;

    long expectedRecordOffsetInFile = PersistentFSRecordsOverLockFreePagedStorage.HEADER_SIZE;
    for (int recordId = NULL_ID + 1; recordId < enoughRecords; recordId++) {
@@ -102,16 +81,15 @@ public class PersistentFSRecordsStorageOverLockFreePagedStorageTest
      );

      expectedRecordOffsetInFile += RECORD_SIZE_IN_BYTES;
-      if (pageSize - (expectedRecordOffsetInFile % pageSize) < RECORD_SIZE_IN_BYTES) {
-        expectedRecordOffsetInFile = (expectedRecordOffsetInFile / pageSize + 1) * pageSize;
+      if (PAGE_SIZE - (expectedRecordOffsetInFile % PAGE_SIZE) < RECORD_SIZE_IN_BYTES) {
+        expectedRecordOffsetInFile = (expectedRecordOffsetInFile / PAGE_SIZE + 1) * PAGE_SIZE;
      }
    }
  }

  @Test
  public void loadRecordsCount_IsConsistentWith_recordOffsetInFile() throws IOException {
-    final int pageSize = pagedStorage.getPageSize();
-    final int enoughRecords = (pageSize / RECORD_SIZE_IN_BYTES) * 16;
+    final int enoughRecords = (PAGE_SIZE / RECORD_SIZE_IN_BYTES) * 16;

    for (int recordId = NULL_ID + 1; recordId < enoughRecords; recordId++) {
      final long recordOffsetInFile = storage.recordOffsetInFileUnchecked(recordId);
@@ -128,8 +106,7 @@ public class PersistentFSRecordsStorageOverLockFreePagedStorageTest
  public void loadRecordsCount_ThrowsIOException_IfUnEvenNumberOfRecordsFound() throws IOException {
    //RC: method is very slow, so use 1 random excess for each record, instead of iterating through
    // each excess: [1..RECORD_SIZE_IN_BYTES) on each record
-    final int pageSize = pagedStorage.getPageSize();
-    final int enoughRecords = (pageSize / RECORD_SIZE_IN_BYTES) * 16;
+    final int enoughRecords = (PAGE_SIZE / RECORD_SIZE_IN_BYTES) * 16;

    for (int recordId = NULL_ID + 1; recordId < enoughRecords; recordId++) {
      final long recordOffsetInFile = storage.recordOffsetInFileUnchecked(recordId);