Skip to content
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

[iOS] Parallelize FileOpenPicker.PickMultipleFilesAsync #18605

Open
MartinZikmund opened this issue Oct 28, 2024 · 3 comments · May be fixed by #18886
Open

[iOS] Parallelize FileOpenPicker.PickMultipleFilesAsync #18605

MartinZikmund opened this issue Oct 28, 2024 · 3 comments · May be fixed by #18886
Assignees
Labels
area/storage 📦 Categorizes an issue or PR as relevant to file storage difficulty/starter 🚀 Categorizes an issue for which the difficulty level is reachable by newcomers kind/enhancement New feature or request platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform

Comments

@MartinZikmund
Copy link
Member

What would you like to be added

If the user picks multiple photos on iOS, the processing currently creates a local copy for each of them in sequence. This is inefficient.

Why is this needed

Parallelize to speed up the processing

For which platform

iOS

Anything else we need to know?

No response

@MartinZikmund MartinZikmund added kind/enhancement New feature or request triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform area/storage 📦 Categorizes an issue or PR as relevant to file storage difficulty/starter 🚀 Categorizes an issue for which the difficulty level is reachable by newcomers and removed triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Oct 28, 2024
@MartinZikmund MartinZikmund changed the title Paralellize FileOpenPicker.PickMultipleFilesAsync on iOS [iOS] Paralellize FileOpenPicker.PickMultipleFilesAsync Oct 28, 2024
@MartinZikmund MartinZikmund changed the title [iOS] Paralellize FileOpenPicker.PickMultipleFilesAsync [iOS] Parallelize FileOpenPicker.PickMultipleFilesAsync Oct 28, 2024
@jeromelaban
Copy link
Member

@darenm Can provide more information on what you tried? Thanks!

@jasonpastor
Copy link

@jeromelaban:
Obviously this isn't meant to be a final version :) For example the hack to wait for 100 file copies, since I am testing 100 files, needs to be based on the results length, or maybe even a more clever pattern. The main goal of this code was to test switching to use LoadFileRepresentation (provides a temporary url reference) and a File.Copy. I saw significantly better results on a large set of files 100 files, 222MB total, but I am not done with modifications/testing to validate that it isn't a mirage. Before the change, the load/save was taking about 4.1 seconds. Originally I saw a reduction to about .8 seconds, but I have also had test runs up to about 1.6 seconds. I am also concerned about the temporary files and making sure they get cleaned up properly, since we are testing with even larger batches of data (GBs).

        private async void ConvertPickerResults(PHPickerResult[] results)
        {
            Console.WriteLine("ConvertPickerResults...");
            var providers = results
                .Select(res => res.ItemProvider)
                .Where(provider => provider != null && provider.RegisteredTypeIdentifiers?.Length > 0)
                .ToArray();

            var start = DateTime.Now;
            int count = 0;
            foreach (NSItemProvider provider in providers)
            {
                var identifier = GetIdentifier(provider.RegisteredTypeIdentifiers ?? []) ?? "public.data";
                var extension = GetExtension(identifier);
            
                //var data = await provider.LoadDataRepresentationAsync(identifier);
                // File Representation instead of Data Representation, I assume Data Representation pulls data into memory.
                provider.LoadFileRepresentation(identifier, (url, err) =>
                {
                    // File Representation is only valid for the lifetime of the callback.  Need to copy file to temp directory here.
                    var destinationUrl = NSFileManager.DefaultManager
                        .GetTemporaryDirectory()
                        .Append($"{NSProcessInfo.ProcessInfo.GloballyUniqueString}.{extension}", false);
                
                    //Console.WriteLine($"Identifier: {identifier}");
                    //Console.WriteLine($"Copy {url.Path} to {destinationUrl.Path}");
                
                    File.Copy(url.Path, destinationUrl.Path, true);
                    
                    //&&& Hack: Need to wait until all files are copied
                    if (++count == 100)
                    {
                        var end = DateTime.Now;
                        Console.WriteLine($"Took {(end - start).TotalSeconds} seconds");             
                    }
                });
            }
        
            return;
        }  

@jasonpastor
Copy link

I cleaned it up a little this weekend to remove the hardcoded file count:

        private async Task ConvertPickerResults(PHPickerResult[] results)
        {
            Console.WriteLine("ConvertPickerResults...");
            var providers = results
                .Select(res => res.ItemProvider)
                .Where(provider => provider != null && provider.RegisteredTypeIdentifiers?.Length > 0)
                .ToArray();

            var start = DateTime.Now;
            var copyTasks = new List<Task>();

            foreach (NSItemProvider provider in providers)
            {
                var identifier = GetIdentifier(provider.RegisteredTypeIdentifiers ?? []) ?? "public.data";
                var extension = GetExtension(identifier);
            
                var tcs = new TaskCompletionSource();
                copyTasks.Add(tcs.Task);

                //var data = await provider.LoadDataRepresentationAsync(identifier);
                // File Representation instead of Data Representation, I assume Data Representation pulls data into memory.
                provider.LoadFileRepresentation(identifier, (url, err) =>
                {
                    // File Representation is only valid for the lifetime of the callback.  Need to copy file to temp directory here.
                    var destinationUrl = NSFileManager.DefaultManager
                        .GetTemporaryDirectory()
                        .Append($"{NSProcessInfo.ProcessInfo.GloballyUniqueString}.{extension}", false);
                
                    File.Copy(url.Path, destinationUrl.Path, true);
                    tcs.SetResult();
                });
            }
            
            await Task.WhenAll(copyTasks);
            var end = DateTime.Now;
            Console.WriteLine($"Took {(end - start).TotalSeconds} seconds");
            return;
        }    

@MartinZikmund MartinZikmund linked a pull request Nov 22, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/storage 📦 Categorizes an issue or PR as relevant to file storage difficulty/starter 🚀 Categorizes an issue for which the difficulty level is reachable by newcomers kind/enhancement New feature or request platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants