API: add IWorkspace.write(Map<IFile, byte[]> ...)

[jukzi]

to create multiple IFile in a batch.

For example during clean-build JDT first deletes all output folders and then writes one .class file after the other. Typically many files are written sequentially. However they could be written in parallel if there would be an API.

This change keeps all changes to the workspace single threaded but forwards the IO of creating multiple files to multiple threads.

The single most important use case would be JDT's AbstractImageBuilder.writeClassFileContents()

The speedup on windows is ~ number of cores, when they have hyper threading.

OutOfMemory is not to be feared as the caller has full control how many bytes he passes.

---

discussion welcome.

[github-actions[bot]]

Test Results

 1 734 files  +  405   1 734 suites  +405   1h 26m 55s :stopwatch: + 23m 9s  3 980 tests +    1   3 958 :white_check_mark: +    2   22 :zzz:  -  1  0 :x: ±0  12 537 runs  +2 937  12 373 :white_check_mark: +2 876  164 :zzz: +61  0 :x: ±0 

Results for commit f2d8a7e5. ± Comparison against base commit e3664ce5.

:recycle: This comment has been updated with latest results.

[jukzi]

CI measured a good performance gain (factor 7) also for linux:

!ENTRY org.eclipse.core.tests.resources 1 0 2024-09-11 06:47:50.781
!MESSAGE [_testWritePerformanceBatch_(org.eclipse.core.tests.resources.IFileTest)] setUp
parallel write took:60ms
sequential write took:443ms

does the CI really run with so many cores?

[jukzi]

Windows verification build 2x:

parallel write took:147ms
sequential write took:335ms

macOS verification build 3x:

parallel write took:94ms
sequential write took:267ms

Linux verification build 5x:

parallel write took:40ms
sequential write took:206ms

[HannesWell]

I'm sorry to say this but I wonder if this feature is really worth to become an API and to maintain it as such. This seems like a very specific feature. Are there more places in the SDK besides JDT that would significantly benefit from it? Or if this is something that can't be implemented externally with the same performance? Couldn't one simply batch the changes with a workspace-runnable and just parallelize the writes within it at own discretion, e.g. by using a parallel stream or latest javas structured concurrency or simple threads?

[jukzi]

If you can show how it is possible to actually do two workspace operations at once - even though they are mutal exclusive and the aliasmanager is not even threadsafe - please do. You can take https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/179934 Tries to write some files in parallel (two different projects). Shows that files are NOT written in parallel as a basis of a test to proof it.

[mickaelistria]

I think the expectation would be that calling file.setContents(...) concurrently would in most cases allow to write files in parallel if they are exclusive. Instead of a new API method, aren't there some internal bits we could change to allow that?

[jukzi]

it's one of the fundamentals that each operation is mutual exclusive:

    at org.eclipse.core.internal.jobs.OrderedLock.acquire(OrderedLock.java:87)
    at org.eclipse.core.internal.resources.WorkManager.checkIn(WorkManager.java:125)
    at org.eclipse.core.internal.resources.Workspace.prepareOperation(Workspace.java:2398)

And that's appropriate as the listeners can be sure that one operation happens after another and that they are informed in the thread of the caller. Changing that would break the API. Also think of a single .java file compiles into multiple .class files - all in the same folder. So it's also the same parent Container that is changed multiple times.

[merks]

I think you are saying the writes can't be done in parallel on multiple threads by an external client because each thread needs its own workspace operation and that only by using internals to write each file content can you work around that constraint...

The API seems very restrictive, focused on quite a narrow use case where one must compute all the contents as byte arrays in order to do saves in parallel. Anything that might involve streaming payloads of more significant size is just not support and cannot in any way be support externally. That seems a bit unfortunate.

[laeubi]

P2 Repository API has a IMetadataRepository.executeBatch(IRunnableWithProgress, IProgressMonitor) method that allows batching successive writes into one big operation. I must confess I would have exspected such thing actually happen from WokrspaceRule/Job/...

In any case the speedup might not be because of many threads writing to the same device but more because of the batching of operations as you have noticed that the speedup is not related to the CPU threads.

So maybe something similar would be useful here so one can perform many updates (maybe in parallel threads) and the notifications are then just performed at the end (like in your parallel write API). This would then even not matter what operations are performed, if stream or byte[] are used and so on...

[jukzi]

I agree that it would be cool if everything would just work in parallel or to have a Stream Collector like parallelStream().colllect(Workspace::collectFileContents) but as far as i know that is not possible. On the other hand the proposed API is easy to understand, simple to implement, useful and sufficient for the usecase. It could also be used for other mass file creations like extracting all files of an archive to workspace but i don't plan to work on such as there is no known hotspot in our daily use.

[merks]

I suppose the entire design pattern for this would look pretty much the same (analogous) if in the end it called

FileSystemResourceManager.write(IFile, InputStream, IFileInfo, int, boolean, IProgressMonitor)

instead of

FileSystemResourceManager.write(IFile, byte[], IFileInfo, int, boolean, IProgressMonitor)

It just feels somehow unfortunate that the more general streaming APIs are not supported in favor of a must narrower use case.

[mickaelistria]

it's one of the fundamentals that each operation is mutual exclusive:

prepareOperations are exclusive, but they're mostly asking/waiting for the workspace whether it's ready to support an operation for the given rule. It returns almost immediately if the requested rule is ready to be processed. It's not blocking the whole workspace while the underlying filesystem operation is running, it's only blocking the requested rule. So unless rules are conflicting, prepareOperation returns promptly and further filesystem changes can be running in parallel. So while internal workspace model changes are exclusive, the filesystem operations are supposed to be able to run in parallel.

Here is an example

    @Test
    public void testParallelWorkspaceOperations() throws Exception {
        IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject("testParallel");
        project.create(null);
        project.open(null);
        List<IFile> _30files = new ArrayList<>(30);
        for (int i = 0; i < 30; i++) {
            IFile file = project.getFile("file" + i);
            file.create(new byte[0], 0, null);
            _30files.add(file);
        }
        Instant before = Instant.now();
        List<CompletableFuture<?>> futures = new ArrayList<>(30);
        long timeToProcessMs = 2000;
        for (IFile file : _30files) {
            CompletableFuture<?> future = CompletableFuture.runAsync(() -> {
                try {
                    file.getWorkspace().run(monitor -> {
                        try {
                            Thread.sleep(timeToProcessMs);
                        } catch (Exception ex) {
                            throw new RuntimeException(ex);
                        }
                    }, file, 0, null);
                } catch (Exception ex) {
                    throw new RuntimeException(ex);
                }
            });
            futures.add(future);
        }
        CompletableFuture.allOf(futures.toArray(CompletableFuture[]::new)).join();
        Duration duration = Duration.between(before, Instant.now());
        System.err.println(duration.getSeconds());
        assertTrue(duration.toMillis() < timeToProcessMs * _30files.size());
    }

You'll see that things perform well: the Thread.sleep() are running in parallel, on my machine the test takes 10s for 60s of actual wait. The Thread.sleep() could equally be a Files.write() operationt taking 2s. The workspace can parallelize operation, the important part is the scheduling rule. A similar implementation can be achieved with WorkspaceJobs which allow a scheduling rule too.

JDT could be improved to take advantage of this without a new API. A new API might still be convenient, but I don't think it's necessary for this case. Moreover, the suggested implementation uses a MultiRule of all files, which would prevent any file from being modified is one is already referenced by the active rule. Going finer grain with the smallest possible rule (1 file) would be less blocking than creating and locking bigger rules.

[jukzi]

FileSystemResourceManager.write(IFile, InputStream,

@merks ok, so you suggest to have a IWorkspace.write(Map<IFile, InputStream> ..) as well? Would be doable - But who would use that? If the client cant convert the InputStreaminto a byte array because of memory issues why he should try to write mutliple files at once?

[jukzi]

could equally be a Files.write()

@mickaelistria you could write Files.write() there but it would not work. fixing to an appropriate rule you suggest JDT does something like

    @Test
    public void _testWritePerformanceBatch_() throws CoreException {
        createInWorkspace(projects[0]);
        Map<IFile, byte[]> fileMap2 = new HashMap<>();
        Map<IFile, byte[]> fileMap1 = new HashMap<>();
        for (int i = 0; i < 1000; i++) {
            IFile file = projects[0].getFile("My" + i + ".class");
            removeFromWorkspace(file);
            ((i % 2 == 0) ? fileMap1 : fileMap2).put(file, ("smallFileContent" + i).getBytes());
        }
        {
            long n0 = System.nanoTime();
            List<CompletableFuture<?>> futures = new ArrayList<>();
            for (Entry<IFile, byte[]> e : fileMap2.entrySet()) {
                CompletableFuture<?> future = CompletableFuture.runAsync(() -> {
                    try {
                        IFile file = e.getKey();
                        ISchedulingRule rule = ResourcesPlugin.getWorkspace().getRuleFactory().createRule(file);
                        ResourcesPlugin.getWorkspace().run(monitor -> {
                            file.write(e.getValue(), false, true, false, null);
                        }, rule, 0, null);
                    } catch (Exception ex) {
                        throw new RuntimeException(ex);
                    }
                });
                futures.add(future);
            }
            CompletableFuture.allOf(futures.toArray(CompletableFuture[]::new)).join();
            long n1 = System.nanoTime();
            System.out.println("mickaelistria parallel write took:" + (n1 - n0) / 1_000_000 + "ms");
        }
        {
        long n0 = System.nanoTime();
            for (Entry<IFile, byte[]> e : fileMap2.entrySet()) {
                e.getKey().write(e.getValue(), false, true, false, null);
            }
        long n1 = System.nanoTime();
        System.out.println("sequential write took:" + (n1 - n0) / 1_000_000 + "ms"); 
        }
    }

mickaelistria parallel write took:1429ms
sequential write took:1185ms

Counterproductive. IFile.write does NOT forward the IO in parallel!

[mickaelistria]

Thanks for trying with actual file.write.

IFile.write does NOT forward the IO in parallel!

IMO, this is where investigation should be focused. Do you know what prevents parallel IO works here (now that we've established that begin/prepareOperation are not to blame)?

[jukzi]

begin/prepareOperation are not to blame

Each individual write has its's own prepareOperation. Only with a batch API that could be solved. see https://github.com/eclipse-platform/eclipse.platform/pull/1549/files#diff-1fb86c69fb8845944ed1e086cf8b5b3b6ee725ae01d5bfb3ad8070cdc87e69bdR2852

[mickaelistria]

Each individual write has its's own prepareOperation

OK, I see that with workspace.run(..., rule, ...) there is a getWorkManager().beginUnprotected(); which allows to release the lock taken by prepareOperation/checkIn; and that's why file.setContents(...) cannot be running in parallel. So the underlying workspace tree requires a lock is actually not concurrent, despite existence of scheduling rules ensuring that changes are not conflicting... That would be a good, but challenging, thing to improve!

[jukzi]

So if nobody comes up with a better alternative i would like to continue with this.

[merks]

FileSystemResourceManager.write(IFile, InputStream,

@merks ok, so you suggest to have a IWorkspace.write(Map<IFile, InputStream> ..) as well? Would be doable - But who would use that? If the client cant convert the InputStreaminto a byte array because of memory issues why he should try to write mutliple files at once?

I certainly don't want to block efforts to improve performance so take what I say with a grain of salt...

It seems to me the primary way to update the content of an IFile has been via InputStream and that adding API to support parallel "setContent" would primarily be focused around streaming APIs because of course one can always use ByteArrayInputStream to make use of that. I understand that this might well be less efficient than using byte[] directly, but to achieve parallelism, only a stream approach is actually needed. So it seems odd from an API point of view. I can certainly imagine generators that could produce a stream of content without ever having the entire content in memory and having that content for all files to be written in parallel. A copy or import operation of an entire project might be such an example. Of course there is also the argument "why provide API when maybe no one will use it?"

That's my 2 cents worth...

[jukzi]

Do you miss Streaming of the IFile (Stream) or Streaming of the content (InputStream)- or both? it would be simple to provide an API IWorkspace.write(Stream<Map.Entry<IFile, InputStream>> contentStream, ... ) As it can be converted to call the proposed API. However i don't know how it could be efficiently implemented in parallel, as to get the workspace lock it needs to know all IFile in advance within the calling thread. Streaming only the content as in IWorkspace.write(Map<IFile, InputStream> ..) could be easily implemented as in the proposed API.

[merks]

I feel a bit sheepish because I know you understand this stuff much more deeply than do I and have invested way more effort than I do with my superficial observations...

Mostly my observation is that these are the ways to update the contents:

And that the new API supports only parallelizing the byte[] versions but not the InputStream versions. (I've never noticed the IFileState versions before.) I think supporting InputStream would be sensible and somehow more uniform and as you say, it looks very similar to what you already did. I don't know how others feel about that...

In any case, I will not block whatever decision you make. And thanks for all the effort to make Eclipse faster!

[jukzi]

I would like to get feedback if resources should have it's own ExecutorService for the parallelism or if that should be passed as parameter. And if resources creates an ExecutorService on it's own if it should be kept static or created per request.

Pros for passing as parameter:

• the caller can decide how many threads should be used (JDT for example has other threads running in parallel and would not like them to be thwart

Cons for having a static ExecutorService:

• the calling thread might have another priority then then the ExecutorService Threads, so writing the files might get too high or too low priority.

[merks]

Everyone is so busy. With respect to executors, there are indeed pros and cons of of course. If one needs to pass it in, then each framework that uses this will create their own executor and then one might end up with many, which would be kind of wasteful. Creating one on each request is of course additional overhead and potentially costly per call, but no long term waste. I didn't even think of the priority thing. I don't have a good sense of what's best. 😞