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 all 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Threading.Tasks;
using Flow.Launcher.Plugin.BrowserBookmark.Helper;
using Flow.Launcher.Plugin.BrowserBookmark.Models;
using Microsoft.Data.Sqlite;

namespace Flow.Launcher.Plugin.BrowserBookmark;

Expand All @@ -27,40 +26,27 @@
{
var bookmarks = new List<Bookmark>();
if (!Directory.Exists(browserDataPath)) return bookmarks;

// Watch the entire user data directory for changes to catch journal file writes.
Main.RegisterBrowserDataDirectory(browserDataPath);

var paths = Directory.GetDirectories(browserDataPath);

foreach (var profile in paths)
{
var bookmarkPath = Path.Combine(profile, "Bookmarks");
if (!File.Exists(bookmarkPath))
continue;

// Register bookmark file monitoring (direct call to Main.RegisterBookmarkFile)
try
{
if (File.Exists(bookmarkPath))
{
Main.RegisterBookmarkFile(bookmarkPath);
}
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to register bookmark file monitoring: {bookmarkPath}", ex);
continue;
}

var source = name + (Path.GetFileName(profile) == "Default" ? "" : $" ({Path.GetFileName(profile)})");
var profileBookmarks = LoadBookmarksFromFile(bookmarkPath, source);

// Load favicons after loading bookmarks
if (Main._settings.EnableFavicons)
{
var faviconDbPath = Path.Combine(profile, "Favicons");

Check warning on line 44 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 49 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 +64,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,24 +109,27 @@
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.GetString() ?? string.Empty}");
}
}
}

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

Check warning on line 128 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) =>
if (!File.Exists(dbPath)) return;

FaviconHelper.ExecuteWithTempDb(_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>();
Expand All @@ -142,62 +139,58 @@
{
// 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");
if (string.IsNullOrEmpty(bookmark.Url) || !Uri.TryCreate(bookmark.Url, UriKind.Absolute, out var uri))
return;

using var connection = new SqliteConnection($"Data Source={tempDbPath};Mode=ReadOnly;Pooling=false");

Check failure on line 145 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / build

The type or namespace name 'SqliteConnection' could not be found (are you missing a using directive or an assembly reference?)

Check failure on line 145 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / build

The type or namespace name 'SqliteConnection' could not be found (are you missing a using directive or an assembly reference?)
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
WHERE m.page_url GLOB @pattern
ORDER BY b.width DESC
LIMIT 1";

cmd.Parameters.AddWithValue("@url", $"%{domain}%");
cmd.Parameters.AddWithValue("@pattern", $"http*{uri.Host}/*");

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

var iconId = reader.GetInt64(0).ToString();
var id = 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");
var faviconPath = Path.Combine(_faviconCacheDir, $"chromium_{uri.Host}_{id}.png");

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

bookmark.FaviconPath = faviconPath;
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to extract bookmark favicon: {bookmark.Url}", ex);
Main._context.API.LogException(ClassName, $"Failed to extract favicon for: {bookmark.Url}", ex);
}
finally
{
// Cache connection and clear pool after all operations to avoid issue:
// ObjectDisposedException: Safe handle has been closed.
SqliteConnection.ClearPool(connection);

Check failure on line 192 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / build

The name 'SqliteConnection' does not exist in the current context

Check failure on line 192 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / build

The name 'SqliteConnection' does not exist in the current context
connection.Close();
connection.Dispose();
}
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Flow.Launcher.Plugin.BrowserBookmark.Models;
using System.Collections.Generic;
using System.IO;

namespace Flow.Launcher.Plugin.BrowserBookmark;

Expand Down
Loading
Loading