From c5b40e28e6be80cbb51ba2ddb5175336d12428dc Mon Sep 17 00:00:00 2001 From: Asaf Masa Date: Tue, 10 Oct 2023 10:06:51 +0300 Subject: [PATCH 1/6] feat: add concurrent upscale --- MergerLogic/DataTypes/Data.cs | 65 ++++++++++++++++++------ MergerLogicUnitTests/DataTypes/FSTest.cs | 3 +- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/MergerLogic/DataTypes/Data.cs b/MergerLogic/DataTypes/Data.cs index 21d1b358..a221d38b 100644 --- a/MergerLogic/DataTypes/Data.cs +++ b/MergerLogic/DataTypes/Data.cs @@ -3,6 +3,7 @@ using MergerLogic.Utils; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using System.Collections.Concurrent; using System.Diagnostics; using System.Reflection; using System.Runtime.Serialization; @@ -198,25 +199,57 @@ protected virtual Extent GetExtent() protected virtual Tile? InternalGetLastExistingTile(Coord coords) { - int z = coords.Z; - int baseTileX = coords.X; - int baseTileY = this.ConvertOriginCoord(coords); //dont forget to use the correct origin when overriding this + return Task.Run(() => + { + // get tiles coordinates + int z = coords.Z; + int baseTileX = coords.X; + int baseTileY = this.ConvertOriginCoord(coords); //dont forget to use the correct origin when overriding this + + // Define all tiles coordinates that needs to be requested for upscale + List coordsList = new List(MaxZoomRead - (MaxZoomRead - coords.Z)); + for (int i = z - 1; i >= 0; i--) + { + baseTileX >>= 1; // Divide by 2 + baseTileY >>= 1; // Divide by 2 - Tile? lastTile = null; + coordsList.Add(new Coord(i, baseTileX, baseTileY)); + } + var response = this.InternalGetExistingTile(coordsList.ToArray()); + return response.Result; - // Go over zoom levels until a tile is found (may not find tile) - for (int i = z - 1; i >= 0; i--) - { - baseTileX >>= 1; // Divide by 2 - baseTileY >>= 1; // Divide by 2 + }).Result; + } - lastTile = this.Utils.GetTile(i, baseTileX, baseTileY); - if (lastTile != null) + /// + /// This method requests all tiles that can possibly be used for upscale in parallel + /// + /// + /// Tile that will be used for upscale + private async Task InternalGetExistingTile(Coord[] coordsArray) + { + ConcurrentDictionary zOrderToTileDictionary = new ConcurrentDictionary(); + // get all tiles concurrently + await Parallel.ForEachAsync(coordsArray, async (coord, cancellationToken) => + { + await Task.Run(() => { - break; - } + Tile? tile = this.Utils.GetTile(coord.Z, coord.X, coord.Y); + if (tile != null) + { + zOrderToTileDictionary.TryAdd(coord.Z, tile); + } + }, cancellationToken); + }); + + if (zOrderToTileDictionary.IsEmpty) + { + return null; } - + // Get first valid tile that can be upscaled + List> list = new List>(zOrderToTileDictionary.ToArray()); + var orderedList = list.OrderBy(kvp => kvp.Key); + Tile? lastTile = orderedList.Last().Value; return lastTile; } @@ -266,14 +299,14 @@ public bool TileExists(Coord coord) public Tile? GetCorrespondingTile(Coord coords, bool upscale) { Stopwatch stopwatch = Stopwatch.StartNew(); - this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] start for coord: {coords.ToString()}, upscale: {upscale}"); + this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] start for coord: z:{coords.Z}, x:{coords.X}, y:{coords.Y}, upscale: {upscale}"); Tile? correspondingTile = this.GetTile(coords.Z, coords.X, coords.Y); if (upscale && correspondingTile == null) { correspondingTile = this.GetLastExistingTile(coords); } - this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] end for coord: {coords.ToString()}, upscale: {upscale}"); + this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] end for coord: z:{coords.Z}, x:{coords.X}, y:{coords.Y}, upscale: {upscale}"); stopwatch.Stop(); this._metricsProvider.TotalFetchTimePerTileHistogram(stopwatch.Elapsed.TotalMilliseconds); return correspondingTile; diff --git a/MergerLogicUnitTests/DataTypes/FSTest.cs b/MergerLogicUnitTests/DataTypes/FSTest.cs index 7931b089..be989ca5 100644 --- a/MergerLogicUnitTests/DataTypes/FSTest.cs +++ b/MergerLogicUnitTests/DataTypes/FSTest.cs @@ -348,7 +348,8 @@ public void GetCorrespondingTileWithUpscale(bool isBase, bool isOneXOne, GridOri .Returns(nullTile); } - this._oneXOneConvertorMock.InSequence(sequence) + this._oneXOneConvertorMock + .InSequence(sequence) .Setup(converter => converter.TryFromTwoXOne(It.Is(c => c.Z == 5 && c.X == 2 && c.Y == 3))) .Returns(isValidConversion ? cords => cords : null); } From 0b87767c65c62fcb8bb44a342bbe13832f43c51e Mon Sep 17 00:00:00 2001 From: Asaf Masa Date: Tue, 24 Oct 2023 12:57:44 +0300 Subject: [PATCH 2/6] chore: update by pr comment --- MergerLogic/DataTypes/Data.cs | 61 ++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/MergerLogic/DataTypes/Data.cs b/MergerLogic/DataTypes/Data.cs index cb640ba9..4c545cde 100644 --- a/MergerLogic/DataTypes/Data.cs +++ b/MergerLogic/DataTypes/Data.cs @@ -199,27 +199,52 @@ protected virtual Extent GetExtent() protected virtual Tile? InternalGetLastExistingTile(Coord coords) { - return Task.Run(() => + this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] started for coord: z:{coords.Z}, x:{coords.X}, y:{coords.Y}"); + // get tiles coordinates + int z = coords.Z; + int baseTileX = coords.X; + int baseTileY = this.ConvertOriginCoord(coords); //dont forget to use the correct origin when overriding this + + // Define all tiles coordinates that needs to be requested for upscale + List coordsList = new List(MaxZoomRead - (MaxZoomRead - coords.Z)); + for (int i = z - 1; i >= 0; i--) { - this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] started for coord: z:{coords.Z}, x:{coords.X}, y:{coords.Y}"); - // get tiles coordinates - int z = coords.Z; - int baseTileX = coords.X; - int baseTileY = this.ConvertOriginCoord(coords); //dont forget to use the correct origin when overriding this - - // Define all tiles coordinates that needs to be requested for upscale - List coordsList = new List(MaxZoomRead - (MaxZoomRead - coords.Z)); - for (int i = z - 1; i >= 0; i--) - { - baseTileX >>= 1; // Divide by 2 - baseTileY >>= 1; // Divide by 2 + baseTileX >>= 1; // Divide by 2 + baseTileY >>= 1; // Divide by 2 - coordsList.Add(new Coord(i, baseTileX, baseTileY)); - } - var response = this.InternalGetExistingTile(coordsList.ToArray()); - return response.Result; + coordsList.Add(new Coord(i, baseTileX, baseTileY)); + } - }).Result; + var responseOfGetlastTileAsync = async delegate (Coord[] coordsArray) + { + ConcurrentDictionary zOrderToTileDictionary = new ConcurrentDictionary(); + // get all tiles concurrently + await Parallel.ForEachAsync(coordsArray, async (coord, cancellationToken) => + { + await Task.Run(() => + { + Tile? tile = this.Utils.GetTile(coord.Z, coord.X, coord.Y); + if (tile != null) + { + zOrderToTileDictionary.TryAdd(coord.Z, tile); + } + }, cancellationToken); + }); + + if (zOrderToTileDictionary.IsEmpty) + { + return null; + } + // Get first valid tile that can be upscaled + List> list = new List>(zOrderToTileDictionary.ToArray()); + var orderedList = list.OrderBy(kvp => kvp.Key); + Tile? lastTile = orderedList.Last().Value; + string message = lastTile == null ? "null" : $"z:{lastTile.Z}, x:{lastTile.X}, y:{lastTile.Y}"; + this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] ended, lastTile: {message}"); + return lastTile; + }; + var response = responseOfGetlastTileAsync(coordsList.ToArray()); + return response.Result; } /// From 6ee77565071cf40f7c757c42f01e48fd35c0265b Mon Sep 17 00:00:00 2001 From: Asaf Masa Date: Tue, 24 Oct 2023 13:07:01 +0300 Subject: [PATCH 3/6] chore: add comment --- MergerLogic/DataTypes/Data.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/MergerLogic/DataTypes/Data.cs b/MergerLogic/DataTypes/Data.cs index 4c545cde..e4a018ce 100644 --- a/MergerLogic/DataTypes/Data.cs +++ b/MergerLogic/DataTypes/Data.cs @@ -215,6 +215,7 @@ protected virtual Extent GetExtent() coordsList.Add(new Coord(i, baseTileX, baseTileY)); } + // Async method to request all tiles that can be used for "upscale" concurrently var responseOfGetlastTileAsync = async delegate (Coord[] coordsArray) { ConcurrentDictionary zOrderToTileDictionary = new ConcurrentDictionary(); From 966ae53dd497419c146d2e6361b70d80e6f8c302 Mon Sep 17 00:00:00 2001 From: Asaf Masa Date: Tue, 24 Oct 2023 13:16:08 +0300 Subject: [PATCH 4/6] chore: remove unused method --- MergerLogic/DataTypes/Data.cs | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/MergerLogic/DataTypes/Data.cs b/MergerLogic/DataTypes/Data.cs index e4a018ce..52206c51 100644 --- a/MergerLogic/DataTypes/Data.cs +++ b/MergerLogic/DataTypes/Data.cs @@ -248,40 +248,6 @@ await Task.Run(() => return response.Result; } - /// - /// This method requests all tiles that can possibly be used for upscale in parallel - /// - /// - /// Tile that will be used for upscale - private async Task InternalGetExistingTile(Coord[] coordsArray) - { - ConcurrentDictionary zOrderToTileDictionary = new ConcurrentDictionary(); - // get all tiles concurrently - await Parallel.ForEachAsync(coordsArray, async (coord, cancellationToken) => - { - await Task.Run(() => - { - Tile? tile = this.Utils.GetTile(coord.Z, coord.X, coord.Y); - if (tile != null) - { - zOrderToTileDictionary.TryAdd(coord.Z, tile); - } - }, cancellationToken); - }); - - if (zOrderToTileDictionary.IsEmpty) - { - return null; - } - // Get first valid tile that can be upscaled - List> list = new List>(zOrderToTileDictionary.ToArray()); - var orderedList = list.OrderBy(kvp => kvp.Key); - Tile? lastTile = orderedList.Last().Value; - string message = lastTile == null ? "null" : $"z:{lastTile.Z}, x:{lastTile.X}, y:{lastTile.Y}"; - this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] ended, lastTile: {message}"); - return lastTile; - } - public bool TileExists(Tile tile) { return this.TileExists(tile.GetCoord()); From 971f5ea9dbba5bc280d13490920a6e104d78dccd Mon Sep 17 00:00:00 2001 From: Asaf Masa Date: Tue, 24 Oct 2023 17:35:35 +0300 Subject: [PATCH 5/6] chore: fix cr comments --- MergerLogic/DataTypes/Data.cs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/MergerLogic/DataTypes/Data.cs b/MergerLogic/DataTypes/Data.cs index 52206c51..f951c038 100644 --- a/MergerLogic/DataTypes/Data.cs +++ b/MergerLogic/DataTypes/Data.cs @@ -216,7 +216,7 @@ protected virtual Extent GetExtent() } // Async method to request all tiles that can be used for "upscale" concurrently - var responseOfGetlastTileAsync = async delegate (Coord[] coordsArray) + var getUpscaleTiles = async delegate (Coord[] coordsArray) { ConcurrentDictionary zOrderToTileDictionary = new ConcurrentDictionary(); // get all tiles concurrently @@ -231,21 +231,20 @@ await Task.Run(() => } }, cancellationToken); }); - - if (zOrderToTileDictionary.IsEmpty) - { - return null; - } - // Get first valid tile that can be upscaled - List> list = new List>(zOrderToTileDictionary.ToArray()); - var orderedList = list.OrderBy(kvp => kvp.Key); - Tile? lastTile = orderedList.Last().Value; - string message = lastTile == null ? "null" : $"z:{lastTile.Z}, x:{lastTile.X}, y:{lastTile.Y}"; - this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] ended, lastTile: {message}"); - return lastTile; + return zOrderToTileDictionary.ToArray(); }; - var response = responseOfGetlastTileAsync(coordsList.ToArray()); - return response.Result; + var response = getUpscaleTiles(coordsList.ToArray()); + var tilesResponseArray = response.Result; + if (tilesResponseArray.Length == 0) + { + return null; + } + // Get first valid tile that can be upscaled + var orderedTilesArray = tilesResponseArray.OrderBy(kvp => kvp.Key); + Tile? lastTile = orderedTilesArray.Last().Value; + string message = lastTile == null ? "null" : $"z:{lastTile.Z}, x:{lastTile.X}, y:{lastTile.Y}"; + this._logger.LogDebug($"[{MethodBase.GetCurrentMethod().Name}] ended, lastTile: {message}"); + return lastTile; } public bool TileExists(Tile tile) From cac0053b8a3974585ad20780ca9fa31e730816a3 Mon Sep 17 00:00:00 2001 From: Asaf Masa Date: Tue, 24 Oct 2023 18:07:53 +0300 Subject: [PATCH 6/6] chore: fix cr comments --- MergerLogic/DataTypes/Data.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MergerLogic/DataTypes/Data.cs b/MergerLogic/DataTypes/Data.cs index f951c038..22e4b1e2 100644 --- a/MergerLogic/DataTypes/Data.cs +++ b/MergerLogic/DataTypes/Data.cs @@ -206,7 +206,7 @@ protected virtual Extent GetExtent() int baseTileY = this.ConvertOriginCoord(coords); //dont forget to use the correct origin when overriding this // Define all tiles coordinates that needs to be requested for upscale - List coordsList = new List(MaxZoomRead - (MaxZoomRead - coords.Z)); + List coordsList = new List(coords.Z); for (int i = z - 1; i >= 0; i--) { baseTileX >>= 1; // Divide by 2