-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Improved memory managment and cleanup #11
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| declare namespace Gql { | ||
| export type WxtQueueCtx = import("../dependencies").Dependencies; | ||
| export type WxtQueueCtx = { | ||
| deps: import("../dependencies").Dependencies; | ||
| }; | ||
|
Comment on lines
-2
to
+4
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm now using |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,24 @@ | ||
| import { createIocContainer } from "@aklinker1/zero-ioc"; | ||
| import { createIocContainer, transient } from "@aklinker1/zero-ioc"; | ||
| import { createChromeWebStore } from "./services/chrome-web-store"; | ||
| import { createFirefoxAddonStore } from "./services/firefox-addon-store"; | ||
| import { createEdgeAddonStore } from "./services/edge-addon-store"; | ||
| import type { ExtensionStores } from "./services/extension-stores"; | ||
| import { ExtensionStoreName } from "./enums"; | ||
| import { createRedisCache } from "./services/redis-cache"; | ||
| import { createInMemoryCache } from "./services/in-memory-cache"; | ||
| import { createEdgeApi } from "./services/edge-api"; | ||
| import { createFirefoxApi } from "./services/firefox-api"; | ||
|
|
||
| export const container = createIocContainer() | ||
| .register("chromeWebStore", createChromeWebStore) | ||
| .register("firefoxAddonStore", createFirefoxAddonStore) | ||
| .register("edgeAddonStore", createEdgeAddonStore) | ||
| .register( | ||
| "cache", | ||
| Bun.redis.connected ? createRedisCache : createInMemoryCache, | ||
| ) | ||
| .register("edgeApi", createEdgeApi) | ||
| .register("firefoxApi", createFirefoxApi) | ||
| .register("chromeWebStore", transient(createChromeWebStore)) | ||
| .register("firefoxAddonStore", transient(createFirefoxAddonStore)) | ||
| .register("edgeAddonStore", transient(createEdgeAddonStore)) | ||
|
Comment on lines
+19
to
+21
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Registering these as The data loaders inside each still have a cache, but they are no longer long-lived, and expire after each requesst. Technically, the stores might be created more than once each request if the store is resolved multiple times per-request... they're not, but could be in the future. |
||
| .register( | ||
| "stores", | ||
| (deps) => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| export const rootResolver: Gql.RootResolver = { | ||
| chromeExtension: ({ id }, ctx) => ctx.chromeWebStore.getExtension(id), | ||
| chromeExtensions: ({ ids }, ctx) => ctx.chromeWebStore.getExtensions(ids), | ||
| firefoxAddon: ({ id }, ctx) => ctx.firefoxAddonStore.getExtension(id), | ||
| firefoxAddons: ({ ids }, ctx) => ctx.firefoxAddonStore.getExtensions(ids), | ||
| edgeAddon: ({ id }, ctx) => ctx.edgeAddonStore.getExtension(id), | ||
| edgeAddons: ({ ids }, ctx) => ctx.edgeAddonStore.getExtensions(ids), | ||
| chromeExtension: ({ id }, ctx) => ctx.deps.chromeWebStore.getExtension(id), | ||
| chromeExtensions: ({ ids }, ctx) => | ||
| ctx.deps.chromeWebStore.getExtensions(ids), | ||
| firefoxAddon: ({ id }, ctx) => ctx.deps.firefoxAddonStore.getExtension(id), | ||
| firefoxAddons: ({ ids }, ctx) => | ||
| ctx.deps.firefoxAddonStore.getExtensions(ids), | ||
| edgeAddon: ({ id }, ctx) => ctx.deps.edgeAddonStore.getExtension(id), | ||
| edgeAddons: ({ ids }, ctx) => ctx.deps.edgeAddonStore.getExtensions(ids), | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,5 +2,5 @@ import { createApp } from "@aklinker1/zeta"; | |
| import { container } from "../dependencies"; | ||
|
|
||
| export const contextPlugin = createApp() | ||
| .decorate(container.resolveAll()) | ||
| .decorate({ deps: container.registrations }) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's where I'm actually passing in the proxy object instead of resolving all services once. |
||
| .export(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export interface Cache { | ||
| get<T>(key: string): Promise<T | undefined>; | ||
| set<T>(key: string, value: T): Promise<void>; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,17 @@ | ||
| import type { Cache } from "./cache"; | ||
| import { crawlExtension } from "./chrome-crawler"; | ||
| import { defineExtensionStore, type ExtensionStore } from "./extension-store"; | ||
| import { ExtensionStore } from "./extension-store"; | ||
|
|
||
| export type ChromeWebStore = ExtensionStore<Gql.ChromeExtension>; | ||
|
|
||
| export function createChromeWebStore() { | ||
| return defineExtensionStore((id) => crawlExtension(String(id), "en")); | ||
| export function createChromeWebStore({ | ||
| cache, | ||
| }: { | ||
| cache: Cache; | ||
| }): ChromeWebStore { | ||
| return new ExtensionStore({ | ||
| fetch: (id) => crawlExtension(String(id), "en"), | ||
| cacheKeyPrefix: "chrome-extension-", | ||
| cache, | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,19 @@ | ||
| import { createEdgeApi } from "./edge-api"; | ||
| import { defineExtensionStore, type ExtensionStore } from "./extension-store"; | ||
| import type { Cache } from "./cache"; | ||
| import type { EdgeApi } from "./edge-api"; | ||
| import { ExtensionStore } from "./extension-store"; | ||
|
|
||
| export type EdgeAddonStore = ExtensionStore<Gql.EdgeAddon>; | ||
|
|
||
| export function createEdgeAddonStore() { | ||
| const api = createEdgeApi(); | ||
|
|
||
| return defineExtensionStore((id) => api.getAddon(String(id))); | ||
| export function createEdgeAddonStore({ | ||
| cache, | ||
| edgeApi, | ||
| }: { | ||
| cache: Cache; | ||
| edgeApi: EdgeApi; | ||
| }): EdgeAddonStore { | ||
| return new ExtensionStore({ | ||
| fetch: (id) => edgeApi.getAddon(String(id)), | ||
| cacheKeyPrefix: "edge-addon-", | ||
| cache, | ||
| }); | ||
| } |
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Converted to a class to speed up creating instances of this object now that it's created every request instead of once up top. In general, singleton objects are faster to create with a factory function once, but classes are faster when they need instantiated multiple times, like on every request. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,74 +1,66 @@ | ||
| import { createCachedDataLoader } from "../utils/cache"; | ||
| import { HOUR_MS } from "../utils/time"; | ||
| import DataLoader from "dataloader"; | ||
| import type { Cache } from "./cache"; | ||
|
|
||
| export type ExtensionId = string | number; | ||
|
|
||
| export interface ExtensionStore<TGqlExtension extends Gql.Extension> { | ||
| export class ExtensionStore<TGqlExtension extends Gql.Extension> { | ||
| private dataloader: DataLoader<ExtensionId, TGqlExtension>; | ||
|
|
||
| constructor( | ||
| readonly options: { | ||
| cacheKeyPrefix: string; | ||
| fetch: (id: ExtensionId) => Promise<TGqlExtension | undefined>; | ||
| cache: Cache; | ||
| }, | ||
| ) { | ||
| this.dataloader = new DataLoader<ExtensionId, TGqlExtension>( | ||
| async (ids): Promise<Array<TGqlExtension | Error>> => { | ||
| const results = await Promise.allSettled( | ||
| ids.map(async (id) => { | ||
| const cacheKey = options.cacheKeyPrefix + id; | ||
| const cached = await options.cache.get(cacheKey); | ||
| if (cached) return cached; | ||
|
|
||
| const result = await options.fetch(id); | ||
| if (result) await options.cache.set(cacheKey, result); | ||
|
|
||
| return result; | ||
| }), | ||
| ); | ||
| return results.map((res) => | ||
| res.status === "fulfilled" ? res.value : res.reason, | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Get an extension by it's ID. | ||
| */ | ||
| getExtension: ( | ||
| extensionId: ExtensionId, | ||
| ) => Promise<TGqlExtension | undefined>; | ||
| getExtension(extensionId: ExtensionId): Promise<TGqlExtension> { | ||
| return this.dataloader.load(extensionId); | ||
| } | ||
|
|
||
| /** | ||
| * Get multiple extensions by their IDs. | ||
| */ | ||
| getExtensions: ( | ||
| async getExtensions( | ||
| extensionIds: ExtensionId[], | ||
| ) => Promise<(TGqlExtension | undefined)[]>; | ||
| ): Promise<(TGqlExtension | Error)[]> { | ||
| return this.dataloader.loadMany(extensionIds); | ||
| } | ||
|
|
||
| /** | ||
| * Get a screenshot given an index. | ||
| */ | ||
| getScreenshotUrl( | ||
| async getScreenshotUrl( | ||
| extensionId: ExtensionId, | ||
| screenshotIndex: number, | ||
| ): Promise<string | undefined>; | ||
| } | ||
|
|
||
| export function defineExtensionStore<TGqlExtension extends Gql.Extension>( | ||
| fetch: (id: ExtensionId) => Promise<TGqlExtension | undefined>, | ||
| ): ExtensionStore<TGqlExtension> { | ||
| const loader = createCachedDataLoader<ExtensionId, TGqlExtension | undefined>( | ||
| HOUR_MS, | ||
| async (ids) => { | ||
| const results = await Promise.allSettled(ids.map((id) => fetch(id))); | ||
| return results.map((res) => | ||
| res.status === "fulfilled" ? res.value : undefined, | ||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| const getExtension: ExtensionStore<TGqlExtension>["getExtension"] = ( | ||
| extensionId, | ||
| ) => loader.load(extensionId); | ||
|
|
||
| const getExtensions: ExtensionStore<TGqlExtension>["getExtensions"] = async ( | ||
| extensionIds, | ||
| ) => { | ||
| const result = await loader.loadMany(extensionIds); | ||
| return result.map((item, index) => { | ||
| if (item instanceof Error) { | ||
| console.warn("Error loading extension:", extensionIds[index], item); | ||
| return undefined; | ||
| } | ||
| return item; | ||
| }); | ||
| }; | ||
|
|
||
| const getScreenshotUrl: ExtensionStore<TGqlExtension>["getScreenshotUrl"] = | ||
| async (extensionId, screenshotIndex) => { | ||
| const extension = await getExtension(extensionId); | ||
| const screenshot = extension?.screenshots.find( | ||
| (screenshot) => screenshot.index == screenshotIndex, | ||
| ); | ||
| return screenshot?.rawUrl; | ||
| }; | ||
|
|
||
| return { | ||
| getExtension, | ||
| getExtensions, | ||
| getScreenshotUrl, | ||
| }; | ||
| ): Promise<string | undefined> { | ||
| const extension = await this.getExtension(extensionId); | ||
| const screenshot = extension.screenshots.find( | ||
| (screenshot) => screenshot.index == screenshotIndex, | ||
| ); | ||
| return screenshot?.rawUrl; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,19 @@ | ||
| import { createFirefoxApi } from "./firefox-api"; | ||
| import { defineExtensionStore, type ExtensionStore } from "./extension-store"; | ||
| import type { Cache } from "./cache"; | ||
| import { ExtensionStore } from "./extension-store"; | ||
| import type { FirefoxApi } from "./firefox-api"; | ||
|
|
||
| export type FirefoxAddonStore = ExtensionStore<Gql.FirefoxAddon>; | ||
|
|
||
| export function createFirefoxAddonStore() { | ||
| const api = createFirefoxApi(); | ||
|
|
||
| return defineExtensionStore((id) => api.getAddon(id)); | ||
| export function createFirefoxAddonStore({ | ||
| cache, | ||
| firefoxApi, | ||
| }: { | ||
| cache: Cache; | ||
| firefoxApi: FirefoxApi; | ||
| }): FirefoxAddonStore { | ||
| return new ExtensionStore({ | ||
| fetch: (id) => firefoxApi.getAddon(String(id)), | ||
| cacheKeyPrefix: "firefox-addon-", | ||
| cache, | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { HOUR_MS } from "../utils/time"; | ||
| import type { Cache } from "./cache"; | ||
|
|
||
| const TTL = HOUR_MS; | ||
|
|
||
| export function createInMemoryCache(): Cache { | ||
| let cache: Record<string, any> = Object.create(null); | ||
| let ttl: Record<string, number> = Object.create(null); | ||
|
|
||
| return { | ||
| get: async (key: string) => { | ||
| if (ttl[key] && Date.now() > ttl[key]) { | ||
| delete cache[key]; | ||
| delete ttl[key]; | ||
| } | ||
| return cache[key]; | ||
| }, | ||
| set: async (key: string, value: any) => { | ||
| cache[key] = value; | ||
| ttl[key] = Date.now() + TTL; | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleaned up my error handling - turns out
graphqlcan acceptErrorinstances as return values, not just when thrown. So this change updates the types to reflect that. This is now thedataloaderpackage handles multiple errors, so this makes it easier to map dataloader outputs to graphql results.