Skip to content

Fix CPU / thread locks + improve performance of BrowserBookmark plugin #3815

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

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 37 additions & 81 deletions Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Text.Json;
using System.Threading.Tasks;
using Flow.Launcher.Plugin.BrowserBookmark.Helper;
using Flow.Launcher.Plugin.BrowserBookmark.Models;
using Microsoft.Data.Sqlite;
Expand Down Expand Up @@ -55,12 +53,12 @@
// Load favicons after loading bookmarks
if (Main._settings.EnableFavicons)
{
var faviconDbPath = Path.Combine(profile, "Favicons");

Check warning on line 56 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
if (File.Exists(faviconDbPath))
{
Main._context.API.StopwatchLogInfo(ClassName, $"Load {profileBookmarks.Count} favicons cost", () =>
{
LoadFaviconsFromDb(faviconDbPath, profileBookmarks);

Check warning on line 61 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
});
}
}
Expand All @@ -78,10 +76,18 @@
if (!File.Exists(path))
return bookmarks;

using var jsonDocument = JsonDocument.Parse(File.ReadAllText(path));
if (!jsonDocument.RootElement.TryGetProperty("roots", out var rootElement))
return bookmarks;
EnumerateRoot(rootElement, bookmarks, source);
try
{
using var jsonDocument = JsonDocument.Parse(File.ReadAllText(path));
if (!jsonDocument.RootElement.TryGetProperty("roots", out var rootElement))
return bookmarks;
EnumerateRoot(rootElement, bookmarks, source);
}
catch (JsonException e)
{
Main._context.API.LogException(ClassName, $"Failed to parse bookmarks file: {path}", e);
}

return bookmarks;
}

Expand Down Expand Up @@ -115,91 +121,41 @@
case "workspace": // Edge Workspace
EnumerateFolderBookmark(subElement, bookmarks, source);
break;
default:
bookmarks.Add(new Bookmark(
subElement.GetProperty("name").GetString(),
subElement.GetProperty("url").GetString(),
source));
case "url":
if (subElement.TryGetProperty("name", out var name) &&
subElement.TryGetProperty("url", out var url))
{
bookmarks.Add(new Bookmark(name.GetString(), url.GetString(), source));
}
break;
}
}
else
{
Main._context.API.LogError(ClassName, $"type property not found for {subElement.GetString()}");
Main._context.API.LogError(ClassName, $"type property not found for {subElement.ToString()}");
}
}
}

private void LoadFaviconsFromDb(string dbPath, List<Bookmark> bookmarks)

Check warning on line 140 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
{
FaviconHelper.LoadFaviconsFromDb(_faviconCacheDir, dbPath, (tempDbPath) =>
{
// Since some bookmarks may have same favicon id, we need to record them to avoid duplicates
var savedPaths = new ConcurrentDictionary<string, bool>();

// Get favicons based on bookmarks concurrently
Parallel.ForEach(bookmarks, bookmark =>
{
// Use read-only connection to avoid locking issues
// Do not use pooling so that we do not need to clear pool: https://github.com/dotnet/efcore/issues/26580
var connection = new SqliteConnection($"Data Source={tempDbPath};Mode=ReadOnly;Pooling=false");
connection.Open();

try
{
var url = bookmark.Url;
if (string.IsNullOrEmpty(url)) return;

// Extract domain from URL
if (!Uri.TryCreate(url, UriKind.Absolute, out Uri uri))
return;

var domain = uri.Host;

using var cmd = connection.CreateCommand();
cmd.CommandText = @"
SELECT f.id, b.image_data
FROM favicons f
JOIN favicon_bitmaps b ON f.id = b.icon_id
JOIN icon_mapping m ON f.id = m.icon_id
WHERE m.page_url LIKE @url
ORDER BY b.width DESC
LIMIT 1";

cmd.Parameters.AddWithValue("@url", $"%{domain}%");

using var reader = cmd.ExecuteReader();
if (!reader.Read() || reader.IsDBNull(1))
return;

var iconId = reader.GetInt64(0).ToString();
var imageData = (byte[])reader["image_data"];

if (imageData is not { Length: > 0 })
return;

var faviconPath = Path.Combine(_faviconCacheDir, $"chromium_{domain}_{iconId}.png");

// Filter out duplicate favicons
if (savedPaths.TryAdd(faviconPath, true))
{
FaviconHelper.SaveBitmapData(imageData, faviconPath);
}

bookmark.FaviconPath = faviconPath;
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to extract bookmark favicon: {bookmark.Url}", ex);
}
finally
{
// Cache connection and clear pool after all operations to avoid issue:
// ObjectDisposedException: Safe handle has been closed.
connection.Close();
connection.Dispose();
}
});
});
const string sql = @"
SELECT f.id, b.image_data
FROM favicons f
JOIN favicon_bitmaps b ON f.id = b.icon_id
JOIN icon_mapping m ON f.id = m.icon_id
WHERE m.page_url GLOB @pattern
ORDER BY b.width DESC
LIMIT 1";

FaviconHelper.ProcessFavicons(

Check warning on line 151 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
dbPath,
_faviconCacheDir,
bookmarks,
sql,
"http*",
reader => (reader.GetInt64(0).ToString(), (byte[])reader["image_data"]),
(uri, id, data) => Path.Combine(_faviconCacheDir, $"chromium_{uri.Host}_{id}.png")
);
}
}
Loading
Loading