From b2e477e6e0d678aeaa39f8e4f18edf2a9e013ea2 Mon Sep 17 00:00:00 2001 From: Matthias Dittrich Date: Sun, 9 Jul 2017 18:17:01 +0200 Subject: [PATCH] Improve speed of myget - Remove magic strings for known nuget sources - Improve error handling of new system.net.http library - Do not convert v3 sources to v2 sources when parsing them -> this lead to a huge performance bottleneck for myget. --- src/Paket.Core/Common/Async.fs | 46 +++++++++- src/Paket.Core/Common/Utils.fs | 41 ++++++--- src/Paket.Core/Dependencies/NuGet.fs | 97 +++++++++++---------- src/Paket.Core/Dependencies/NuGetCache.fs | 60 +++++++------ src/Paket.Core/Dependencies/NuGetV2.fs | 8 +- src/Paket.Core/Dependencies/NuGetV3.fs | 6 +- src/Paket.Core/Versioning/PackageSources.fs | 23 ++++- src/Paket/Paket.fsproj | 2 +- 8 files changed, 190 insertions(+), 93 deletions(-) diff --git a/src/Paket.Core/Common/Async.fs b/src/Paket.Core/Common/Async.fs index 888f515582..a440e82095 100644 --- a/src/Paket.Core/Common/Async.fs +++ b/src/Paket.Core/Common/Async.fs @@ -14,7 +14,20 @@ module AsyncExtensions = open System open System.Threading.Tasks open System.Threading - + open System.Runtime.ExceptionServices + + // This uses a trick to get the underlying OperationCanceledException + let inline getCancelledException (completedTask:Task) (waitWithAwaiter) = + let fallback = new TaskCanceledException(completedTask) :> OperationCanceledException + // sadly there is no other public api to retrieve it, but to call .GetAwaiter().GetResult(). + try waitWithAwaiter() + // should not happen, but just in case... + fallback + with + | :? OperationCanceledException as o -> o + | other -> + // shouldn't happen, but just in case... + new TaskCanceledException(fallback.Message, other) :> OperationCanceledException type Microsoft.FSharp.Control.Async with /// Runs both computations in parallel and returns the result as a tuple. static member Parallel (a : Async<'a>, b : Async<'b>) : Async<'a * 'b> = @@ -25,6 +38,37 @@ module AsyncExtensions = let! b'' = b' return (a'',b'') } + static member AwaitTaskWithoutAggregate (task:Task<'T>) : Async<'T> = + Async.FromContinuations(fun (cont, econt, ccont) -> + let continuation (completedTask : Task<_>) = + if completedTask.IsCanceled then + let cancelledException = + getCancelledException completedTask (fun () -> completedTask.GetAwaiter().GetResult() |> ignore) + econt (cancelledException) + elif completedTask.IsFaulted then + if completedTask.Exception.InnerExceptions.Count = 1 then + econt completedTask.Exception.InnerExceptions.[0] + else + econt completedTask.Exception + else + cont completedTask.Result + task.ContinueWith(Action>(continuation)) |> ignore) + static member AwaitTaskWithoutAggregate (task:Task) : Async = + Async.FromContinuations(fun (cont, econt, ccont) -> + let continuation (completedTask : Task) = + if completedTask.IsCanceled then + let cancelledException = + getCancelledException completedTask (fun () -> completedTask.GetAwaiter().GetResult() |> ignore) + econt (cancelledException) + elif completedTask.IsFaulted then + if completedTask.Exception.InnerExceptions.Count = 1 then + econt completedTask.Exception.InnerExceptions.[0] + else + econt completedTask.Exception + else + cont () + task.ContinueWith(Action(continuation)) |> ignore) + static member awaitTaskWithToken (fallBack:unit -> 'T) (item: Task<'T>) : Async<'T> = async { let! ct = Async.CancellationToken diff --git a/src/Paket.Core/Common/Utils.fs b/src/Paket.Core/Common/Utils.fs index 75a4262ba2..85d9bb591d 100644 --- a/src/Paket.Core/Common/Utils.fs +++ b/src/Paket.Core/Common/Utils.fs @@ -309,7 +309,6 @@ let normalizeFeedUrl (source:string) = | "http://nuget.org/api/v2" -> Constants.DefaultNuGetStream.Replace("https","http") | "https://www.nuget.org/api/v2" -> Constants.DefaultNuGetStream | "http://www.nuget.org/api/v2" -> Constants.DefaultNuGetStream.Replace("https","http") - | url when url.EndsWith("/api/v3/index.json") -> url.Replace("/api/v3/index.json","") | source -> source #if NETSTANDARD1_6 @@ -385,12 +384,24 @@ let getDefaultProxyFor = | Some p -> if p.GetProxy uri <> uri then p else getDefault() | None -> getDefault()) + +exception RequestReturnedError of statusCode:HttpStatusCode * content:Stream * mediaType:string +let failIfNoSuccess (resp:HttpResponseMessage) = async { + if not resp.IsSuccessStatusCode then + if verbose then + tracefn "Request failed with '%d': '%s'" (int resp.StatusCode) (resp.RequestMessage.RequestUri.ToString()) + let mem = new MemoryStream() + do! resp.Content.CopyToAsync(mem) |> Async.AwaitTaskWithoutAggregate + mem.Position <- 0L + raise <| RequestReturnedError(resp.StatusCode, mem, resp.Content.Headers.ContentType.MediaType) + () } type HttpClient with member x.DownloadFileTaskAsync (uri : Uri, tok : CancellationToken, filePath : string) = async { - let! response = x.GetAsync(uri, tok) |> Async.AwaitTask + let! response = x.GetAsync(uri, tok) |> Async.AwaitTaskWithoutAggregate + do! failIfNoSuccess response use fileStream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None) - do! response.Content.CopyToAsync(fileStream) |> Async.AwaitTask + do! response.Content.CopyToAsync(fileStream) |> Async.AwaitTaskWithoutAggregate fileStream.Flush() } |> Async.StartAsTask member x.DownloadFileTaskAsync (uri : string, tok : CancellationToken, filePath : string) = x.DownloadFileTaskAsync(Uri uri, tok, filePath) @@ -400,8 +411,9 @@ type HttpClient with x.DownloadFileTaskAsync(uri, CancellationToken.None, filePath).GetAwaiter().GetResult() member x.DownloadStringTaskAsync (uri : Uri, tok : CancellationToken) = async { - let! response = x.GetAsync(uri, tok) |> Async.AwaitTask - let! result = response.Content.ReadAsStringAsync() |> Async.AwaitTask + let! response = x.GetAsync(uri, tok) |> Async.AwaitTaskWithoutAggregate + do! failIfNoSuccess response + let! result = response.Content.ReadAsStringAsync() |> Async.AwaitTaskWithoutAggregate return result } |> Async.StartAsTask member x.DownloadStringTaskAsync (uri : string, tok : CancellationToken) = x.DownloadStringTaskAsync(Uri uri, tok) @@ -412,8 +424,9 @@ type HttpClient with member x.DownloadDataTaskAsync(uri : Uri, tok : CancellationToken) = async { - let! response = x.GetAsync(uri, tok) |> Async.AwaitTask - let! result = response.Content.ReadAsByteArrayAsync() |> Async.AwaitTask + let! response = x.GetAsync(uri, tok) |> Async.AwaitTaskWithoutAggregate + do! failIfNoSuccess response + let! result = response.Content.ReadAsByteArrayAsync() |> Async.AwaitTaskWithoutAggregate return result } |> Async.StartAsTask member x.DownloadDataTaskAsync (uri : string, tok : CancellationToken) = x.DownloadDataTaskAsync(Uri uri, tok) @@ -491,7 +504,7 @@ let downloadFromUrl (auth:Auth option, url : string) (filePath: string) = if verbose then verbosefn "Starting download from '%O'" url use _ = Profile.startCategory Profile.Category.NuGetDownload - let task = client.DownloadFileTaskAsync (Uri url, tok, filePath) |> Async.AwaitTask + let task = client.DownloadFileTaskAsync (Uri url, tok, filePath) |> Async.AwaitTaskWithoutAggregate do! task with | exn -> @@ -510,7 +523,7 @@ let getFromUrl (auth:Auth option, url : string, contentType : string) = if verbose then verbosefn "Starting request to '%O'" url use _ = Profile.startCategory Profile.Category.NuGetRequest - return! client.DownloadStringTaskAsync (Uri url, tok) |> Async.AwaitTask + return! client.DownloadStringTaskAsync (Uri url, tok) |> Async.AwaitTaskWithoutAggregate with | exn -> return raise <| Exception(sprintf "Could not retrieve data from '%s'" url, exn) @@ -530,7 +543,7 @@ let getXmlFromUrl (auth:Auth option, url : string) = if verbose then verbosefn "Starting request to '%O'" url use _ = Profile.startCategory Profile.Category.NuGetRequest - return! client.DownloadStringTaskAsync (Uri url, tok) |> Async.AwaitTask + return! client.DownloadStringTaskAsync (Uri url, tok) |> Async.AwaitTaskWithoutAggregate with | exn -> return raise <| Exception(sprintf "Could not retrieve data from '%s'" url, exn) @@ -569,12 +582,12 @@ let safeGetFromUrl (auth:Auth option, url : string, contentType : string) = if verbose then verbosefn "Starting request to '%O'" uri use _ = Profile.startCategory Profile.Category.NuGetRequest - let! raw = client.DownloadStringTaskAsync(uri, tok) |> Async.awaitTaskWithToken (fun () -> failwithf "Uri '%O' failed to respond and cancellation was requested." uri) + let! raw = client.DownloadStringTaskAsync(uri, tok) |> Async.AwaitTaskWithoutAggregate return SuccessResponse raw with - | :? WebException as w -> - match w.Response with - | :? HttpWebResponse as wr when wr.StatusCode = HttpStatusCode.NotFound -> return NotFound + | RequestReturnedError(statusCode, content, mediaType) as w -> + match statusCode with + | HttpStatusCode.NotFound -> return NotFound | _ -> if verbose then Logging.verbosefn "Error while retrieving '%s': %O" url w diff --git a/src/Paket.Core/Dependencies/NuGet.fs b/src/Paket.Core/Dependencies/NuGet.fs index faf6f0dff2..4ccc18d4e0 100644 --- a/src/Paket.Core/Dependencies/NuGet.fs +++ b/src/Paket.Core/Dependencies/NuGet.fs @@ -121,6 +121,12 @@ let tryNuGetV3 (auth, nugetV3Url, package:PackageName) = let rec private getPackageDetails alternativeProjectRoot root force (sources:PackageSource list) packageName (version:SemVerInfo) : Async = async { + let inCache = + sources + |> Seq.choose(fun source -> + NuGetCache.tryGetDetailsFromCache force source.Url packageName version |> Option.map (fun details -> source, details)) + |> Seq.tryHead + let tryV2 (nugetSource:NugetSource) force = NuGetV2.getDetailsFromNuGet force @@ -156,30 +162,23 @@ let rec private getPackageDetails alternativeProjectRoot root force (sources:Pac | ODataSearchResult.EmptyResult -> return! trySelectFirst rest | [] -> return None } - sources - |> List.sortBy (fun source -> - match source with // put local caches to the end - | LocalNuGet(_,Some _) -> true - | _ -> false) - |> List.map (fun source -> source, async { - try + match inCache with + | Some (source, ODataSearchResult.Match result) -> async { return Some (source, result) } + | _ -> + sources + |> List.sortBy (fun source -> + // put local caches to the end + // prefer nuget gallery match source with - | NuGetV2 nugetSource -> - return! tryV2 nugetSource force - | NuGetV3 nugetSource when NuGetV2.urlSimilarToTfsOrVsts nugetSource.Url -> - match NuGetV3.calculateNuGet2Path nugetSource.Url with - | Some url -> - let nugetSource : NugetSource = - { Url = url - Authentication = nugetSource.Authentication } + | LocalNuGet(_,Some _) -> 10 + | s when s.NuGetType = KnownNuGetSources.OfficialNuGetGallery -> 1 + | _ -> 3) + |> List.map (fun source -> source, async { + try + match source with + | NuGetV2 nugetSource -> return! tryV2 nugetSource force - | _ -> - return! tryV3 nugetSource force - | NuGetV3 nugetSource -> - try - return! tryV3 nugetSource force - with - | exn -> + | NuGetV3 nugetSource when urlSimilarToTfsOrVsts nugetSource.Url -> match NuGetV3.calculateNuGet2Path nugetSource.Url with | Some url -> let nugetSource : NugetSource = @@ -187,24 +186,37 @@ let rec private getPackageDetails alternativeProjectRoot root force (sources:Pac Authentication = nugetSource.Authentication } return! tryV2 nugetSource force | _ -> - raise exn return! tryV3 nugetSource force - - | LocalNuGet(path,hasCache) -> - return! NuGetLocal.getDetailsFromLocalNuGetPackage hasCache.IsSome alternativeProjectRoot root path packageName version - with - | :? System.IO.IOException as exn -> - // Handling IO exception here for less noise in output: https://github.com/fsprojects/Paket/issues/2480 - if verbose then - traceWarnfn "I/O error for source '%O': %O" source exn - else - traceWarnfn "I/O error for source '%O': %s" source exn.Message - return EmptyResult - | e -> - traceWarnfn "Source '%O' exception: %O" source e - //let capture = ExceptionDispatchInfo.Capture e - return EmptyResult }) - |> trySelectFirst + | NuGetV3 nugetSource -> + try + return! tryV3 nugetSource force + with + | exn -> + match NuGetV3.calculateNuGet2Path nugetSource.Url with + | Some url -> + let nugetSource : NugetSource = + { Url = url + Authentication = nugetSource.Authentication } + return! tryV2 nugetSource force + | _ -> + raise exn + return! tryV3 nugetSource force + + | LocalNuGet(path,hasCache) -> + return! NuGetLocal.getDetailsFromLocalNuGetPackage hasCache.IsSome alternativeProjectRoot root path packageName version + with + | :? System.IO.IOException as exn -> + // Handling IO exception here for less noise in output: https://github.com/fsprojects/Paket/issues/2480 + if verbose then + traceWarnfn "I/O error for source '%O': %O" source exn + else + traceWarnfn "I/O error for source '%O': %s" source exn.Message + return EmptyResult + | e -> + traceWarnfn "Source '%O' exception: %O" source e + //let capture = ExceptionDispatchInfo.Capture e + return EmptyResult }) + |> trySelectFirst let! maybePackageDetails = getPackageDetails force let! source,nugetObject = @@ -347,11 +359,8 @@ let GetVersions force alternativeProjectRoot root (sources, packageName:PackageN | Some v3Url -> return (getVersionsCached "V3" tryNuGetV3 (nugetSource, auth, v3Url, packageName)) :: v2Feeds | NuGetV3 source -> let! versionsAPI = PackageSources.getNuGetV3Resource source AllVersionsAPI - let req = tryNuGetV3 - (source.Authentication |> Option.map toBasicAuth, - versionsAPI, - packageName) - return [ req ] + let auth = source.Authentication |> Option.map toBasicAuth + return [ getVersionsCached "V3" tryNuGetV3 (nugetSource, auth, versionsAPI, packageName) ] | LocalNuGet(path,Some _) -> return [ NuGetLocal.getAllVersionsFromLocalPath (true, path, packageName, alternativeProjectRoot, root) ] | LocalNuGet(path,None) -> diff --git a/src/Paket.Core/Dependencies/NuGetCache.fs b/src/Paket.Core/Dependencies/NuGetCache.fs index 04831c4769..82e5a00b32 100644 --- a/src/Paket.Core/Dependencies/NuGetCache.fs +++ b/src/Paket.Core/Dependencies/NuGetCache.fs @@ -166,6 +166,37 @@ module ODataSearchResult = match x with | EmptyResult -> failwithf "Cannot call get on 'EmptyResult'" | Match r -> r + +let tryGetDetailsFromCache force nugetURL (packageName:PackageName) (version:SemVerInfo) : ODataSearchResult option = + let cacheFile, oldFiles = getCacheFiles NuGetPackageCache.CurrentCacheVersion nugetURL packageName version + oldFiles |> Seq.iter (fun f -> File.Delete f) + if not force && cacheFile.Exists then + let json = File.ReadAllText(cacheFile.FullName) + let cacheResult = + try + let cachedObject = JsonConvert.DeserializeObject json + if (PackageName cachedObject.PackageName <> packageName) || + (cachedObject.Version <> version.Normalize()) + then + traceVerbose (sprintf "Invalidating Cache '%s:%s' <> '%s:%s'" cachedObject.PackageName cachedObject.Version packageName.Name (version.Normalize())) + cacheFile.Delete() + None + else + Some cachedObject + with + | exn -> + cacheFile.Delete() + if verbose then + traceWarnfn "Error while loading cache: %O" exn + else + traceWarnfn "Error while loading cache: %s" exn.Message + None + match cacheResult with + | Some res -> Some (ODataSearchResult.Match res) + | None -> None + else + None + let getDetailsFromCacheOr force nugetURL (packageName:PackageName) (version:SemVerInfo) (get : unit -> ODataSearchResult Async) : ODataSearchResult Async = let cacheFile, oldFiles = getCacheFiles NuGetPackageCache.CurrentCacheVersion nugetURL packageName version oldFiles |> Seq.iter (fun f -> File.Delete f) @@ -181,32 +212,9 @@ let getDetailsFromCacheOr force nugetURL (packageName:PackageName) (version:SemV return result } async { - if not force && cacheFile.Exists then - let json = File.ReadAllText(cacheFile.FullName) - let cacheResult = - try - let cachedObject = JsonConvert.DeserializeObject json - if (PackageName cachedObject.PackageName <> packageName) || - (cachedObject.Version <> version.Normalize()) - then - traceVerbose (sprintf "Invalidating Cache '%s:%s' <> '%s:%s'" cachedObject.PackageName cachedObject.Version packageName.Name (version.Normalize())) - cacheFile.Delete() - None - else - Some cachedObject - with - | exn -> - cacheFile.Delete() - if verbose then - traceWarnfn "Error while loading cache: %O" exn - else - traceWarnfn "Error while loading cache: %s" exn.Message - None - match cacheResult with - | Some res -> return ODataSearchResult.Match res - | None -> return! get() - else - return! get() + match tryGetDetailsFromCache force nugetURL packageName version with + | None -> return! get() + | Some res -> return res } diff --git a/src/Paket.Core/Dependencies/NuGetV2.fs b/src/Paket.Core/Dependencies/NuGetV2.fs index ed9079c814..c7df8121da 100644 --- a/src/Paket.Core/Dependencies/NuGetV2.fs +++ b/src/Paket.Core/Dependencies/NuGetV2.fs @@ -255,8 +255,6 @@ let getDetailsFromNuGetViaODataFast auth nugetURL (packageName:PackageName) (ver return! fallback() } -let urlSimilarToTfsOrVsts url = - String.containsIgnoreCase "visualstudio.com" url || (String.containsIgnoreCase "/_packaging/" url && String.containsIgnoreCase "/nuget/v" url) /// Gets package details from NuGet via OData let getDetailsFromNuGetViaOData auth nugetURL (packageName:PackageName) (version:SemVerInfo) = @@ -270,9 +268,9 @@ let getDetailsFromNuGetViaOData auth nugetURL (packageName:PackageName) (version | SafeWebResult.SuccessResponse r -> async { return Some r } | SafeWebResult.NotFound -> async { return None } | SafeWebResult.UnknownError err when - String.containsIgnoreCase "myget.org" nugetURL || - String.containsIgnoreCase "nuget.org" nugetURL || - String.containsIgnoreCase "visualstudio.com" nugetURL -> + urlIsMyGet nugetURL || + urlIsNugetGallery nugetURL || + urlSimilarToTfsOrVsts nugetURL -> raise <| System.Exception( sprintf "Could not get package details for %O from %s" packageName nugetURL, diff --git a/src/Paket.Core/Dependencies/NuGetV3.fs b/src/Paket.Core/Dependencies/NuGetV3.fs index d4703d2a4c..8065097eda 100644 --- a/src/Paket.Core/Dependencies/NuGetV3.fs +++ b/src/Paket.Core/Dependencies/NuGetV3.fs @@ -131,7 +131,11 @@ let FindAutoCompleteVersionsForPackage(nugetURL, auth, package, includingPrerele let internal findVersionsForPackage(v3Url, auth, packageName:Domain.PackageName) = - let url = sprintf "%s%O/index.json?semVerLevel=2.0.0" v3Url packageName + // Comment from http://api.nuget.org/v3/index.json + // explicitely says + // Base URL of Azure storage where NuGet package registration info for NET Core is stored, in the format https://api.nuget.org/v3-flatcontainer/{id-lower}/{id-lower}.{version-lower}.nupkg + // so I guess we need to take "id-lower" here -> myget actually needs tolower + let url = sprintf "%s%s/index.json?semVerLevel=2.0.0" v3Url (packageName.CompareString) NuGetRequestGetVersions.ofSimpleFunc url (fun _ -> async { let! response = safeGetFromUrl(auth,url,acceptJson) // NuGet is showing old versions first diff --git a/src/Paket.Core/Versioning/PackageSources.fs b/src/Paket.Core/Versioning/PackageSources.fs index 1afb62b345..13ef7e595d 100644 --- a/src/Paket.Core/Versioning/PackageSources.fs +++ b/src/Paket.Core/Versioning/PackageSources.fs @@ -57,6 +57,22 @@ let RemoveOutsideQuotes(path : string) = let trimChars = [|'\"'|] path.Trim(trimChars) +let urlSimilarToTfsOrVsts url = + String.containsIgnoreCase "visualstudio.com" url || (String.containsIgnoreCase "/_packaging/" url && String.containsIgnoreCase "/nuget/v" url) + +let urlIsNugetGallery url = + String.containsIgnoreCase "nuget.org" url + +let urlIsMyGet url = + String.containsIgnoreCase "myget.org" url + +type KnownNuGetSources = + | OfficialNuGetGallery + | TfsOrVsts + | MyGet + | UnknownNuGetServer + + type NugetSource = { Url : string Authentication : NugetSourceAuthentication option } @@ -166,7 +182,12 @@ type PackageSource = | NuGetV2 source -> source.Url | NuGetV3 source -> source.Url | LocalNuGet(path,_) -> path - + member x.NuGetType = + match x.Url with + | _ when urlIsNugetGallery x.Url -> KnownNuGetSources.OfficialNuGetGallery + | _ when urlIsMyGet x.Url -> KnownNuGetSources.MyGet + | _ when urlSimilarToTfsOrVsts x.Url -> KnownNuGetSources.TfsOrVsts + | _ -> KnownNuGetSources.UnknownNuGetServer static member Parse(line : string) = let sourceRegex = Regex("source[ ]*[\"]([^\"]*)[\"]", RegexOptions.IgnoreCase) let parts = line.Split ' ' diff --git a/src/Paket/Paket.fsproj b/src/Paket/Paket.fsproj index 12a54621a8..1b48cc9d71 100644 --- a/src/Paket/Paket.fsproj +++ b/src/Paket/Paket.fsproj @@ -31,7 +31,7 @@ Project paket.exe Project - update + update --verbose C:\proj\testing\