Skip to content
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

Platform fetch performance improvements #857

Merged
merged 10 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion backend/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@

load_dotenv()

# UVICORN
# GUNICORN
DEV_PORT: Final = int(os.environ.get("VITE_BACKEND_DEV_PORT", "5000"))
DEV_HOST: Final = "0.0.0.0"
ROMM_HOST: Final = os.environ.get("ROMM_HOST", DEV_HOST)
GUNICORN_WORKERS: Final = int(os.environ.get("GUNICORN_WORKERS", 2))

# PATHS
ROMM_BASE_PATH: Final = os.environ.get("ROMM_BASE_PATH", "/romm")
Expand Down
4 changes: 1 addition & 3 deletions backend/endpoints/platform.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ def get_platform(request: Request, id: int) -> PlatformSchema:
PlatformSchema: Platform
"""

return PlatformSchema.from_orm_with_request(
db_platform_handler.get_platforms(id), request
)
return db_platform_handler.get_platforms(id)


@protected_route(router.put, "/platforms/{id}", ["platforms.write"])
Expand Down
16 changes: 1 addition & 15 deletions backend/endpoints/responses/platform.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
from typing import Optional

Check failure on line 1 in backend/endpoints/responses/platform.py

View check run for this annotation

Trunk.io / Trunk Check

isort

Incorrect formatting, autoformat by running 'trunk fmt'
from pydantic import BaseModel, Field
from fastapi import Request

from models.platform import Platform
from .firmware import FirmwareSchema


Expand All @@ -16,19 +14,7 @@
name: str
logo_path: Optional[str] = ""
rom_count: int

firmware_files: list[FirmwareSchema] = Field(default_factory=list)
firmware: list[FirmwareSchema] = Field(default_factory=list)

class Config:
from_attributes = True

@classmethod
def from_orm_with_request(
cls, db_platform: Platform, request: Request
) -> "PlatformSchema":
platform = cls.model_validate(db_platform)
platform.firmware_files = [
FirmwareSchema.model_validate(f)
for f in sorted(db_platform.firmware, key=lambda x: x.file_name)
]
return platform
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer needed since as it turns out performance is unaffected by loading firmware data as well

61 changes: 38 additions & 23 deletions backend/handler/database/platforms_handler.py
Original file line number Diff line number Diff line change
@@ -1,37 +1,58 @@
import functools

Check failure on line 1 in backend/handler/database/platforms_handler.py

View check run for this annotation

Trunk.io / Trunk Check

isort

Incorrect formatting, autoformat by running 'trunk fmt'
from sqlalchemy import delete, or_
from sqlalchemy.orm import Session, Query, joinedload

from decorators.database import begin_session
from models.platform import Platform
from models.rom import Rom
from sqlalchemy import delete, func, or_, select
from sqlalchemy.orm import Session

from .base_handler import DBBaseHandler


def with_query(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
session = kwargs.get("session")
if session is None:
raise ValueError("session is required")

kwargs["query"] = session.query(Platform).options(joinedload(Platform.roms))
return func(*args, **kwargs)

return wrapper


class DBPlatformsHandler(DBBaseHandler):
@begin_session
def add_platform(self, platform: Platform, session: Session = None):
return session.merge(platform)
@with_query
def add_platform(
self, platform: Platform, query: Query = None, session: Session = None
) -> Platform | None:
session.merge(platform)
session.flush()

return query.filter(Platform.fs_slug == platform.fs_slug).first()

@begin_session
def get_platforms(self, id: int = None, session: Session = None):
@with_query
def get_platforms(
self, id: int = None, query: Query = None, session: Session = None
) -> list[Platform] | Platform | None:
return (
session.get(Platform, id)
query.get(id)
if id
else (
session.scalars(select(Platform).order_by(Platform.name.asc()))
.unique()
.all()
)
else (session.scalars(query.order_by(Platform.name.asc())).unique().all())
)

@begin_session
def get_platform_by_fs_slug(self, fs_slug: str, session: Session = None):
return session.scalars(
select(Platform).filter_by(fs_slug=fs_slug).limit(1)
).first()
@with_query
def get_platform_by_fs_slug(
self, fs_slug: str, query: Query = None, session: Session = None
) -> Platform | None:
return session.scalars(query.filter_by(fs_slug=fs_slug).limit(1)).first()

@begin_session
def delete_platform(self, id: int, session: Session = None):
def delete_platform(self, id: int, session: Session = None) -> int:
# Remove all roms from that platforms first
session.execute(
delete(Rom)
Expand All @@ -45,13 +66,7 @@
)

@begin_session
def get_rom_count(self, platform_id: int, session: Session = None):
return session.scalar(
select(func.count()).select_from(Rom).filter_by(platform_id=platform_id)
)

@begin_session
def purge_platforms(self, fs_platforms: list[str], session: Session = None):
def purge_platforms(self, fs_platforms: list[str], session: Session = None) -> int:
return session.execute(
delete(Platform)
.where(or_(Platform.fs_slug.not_in(fs_platforms), Platform.slug.is_(None)))
Expand Down
14 changes: 7 additions & 7 deletions backend/models/platform.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from sqlalchemy import Column, Integer, String, select, func

Check failure on line 1 in backend/models/platform.py

View check run for this annotation

Trunk.io / Trunk Check

isort

Incorrect formatting, autoformat by running 'trunk fmt'
from sqlalchemy.orm import Mapped, relationship, column_property

from models.base import BaseModel
from models.rom import Rom
from models.firmware import Firmware
from sqlalchemy import Column, Integer, String
from sqlalchemy.orm import Mapped, relationship


class Platform(BaseModel):
Expand All @@ -24,11 +25,10 @@
"Firmware", lazy="selectin", back_populates="platform"
)

@property
def rom_count(self) -> int:
from handler.database import db_platform_handler

return db_platform_handler.get_rom_count(self.id)
# This runs a subquery to get the count of roms for the platform
rom_count = column_property(
select(func.count(Rom.id)).where(Rom.platform_id == id).scalar_subquery()
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Significantly faster since it runs in SQL land instead of making another DB query


def __repr__(self) -> str:
return self.name
2 changes: 1 addition & 1 deletion docker/init_scripts/init
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env bash

Check failure on line 1 in docker/init_scripts/init

View check run for this annotation

Trunk.io / Trunk Check

shfmt

Incorrect formatting, autoformat by running 'trunk fmt'

set -o errexit # treat errors as fatal
set -o nounset # treat unset variables as an error
Expand Down Expand Up @@ -46,7 +46,7 @@
--bind=0.0.0.0:5000 \
--bind=unix:/tmp/gunicorn.sock \
--pid=/tmp/gunicorn.pid \
--workers 2 \
--workers ${GUNICORN_WORKERS:=2} \

Check notice on line 49 in docker/init_scripts/init

View check run for this annotation

Trunk.io / Trunk Check

shellcheck(SC2086)

[new] Double quote to prevent globbing and word splitting.
main:app &
}

Expand Down
3 changes: 3 additions & 0 deletions env.template
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
ROMM_BASE_PATH=/path/to/romm_mock
VITE_BACKEND_DEV_PORT=5000

# Gunicorn (optional)
ROMM_HOST=localhost
GUNICORN_WORKERS=4 # (2 × CPU cores) + 1

# IGDB credentials
IGDB_CLIENT_ID=
Expand Down
44 changes: 39 additions & 5 deletions frontend/src/App.vue
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
<script setup lang="ts">
import Notification from "@/components/Notification.vue";
import api from "@/services/api/index";
import userApi from "@/services/api/user";
import platformApi from "@/services/api/platform";
import socket from "@/services/socket";
import storeConfig from "@/stores/config";
import storeGalleryFilter from "@/stores/galleryFilter";
import storeHeartbeat from "@/stores/heartbeat";
import storeRoms, { type Rom } from "@/stores/roms";
import storePlatforms from "@/stores/platforms";
import storeAuth from "@/stores/auth";
import storeScanning from "@/stores/scanning";
import type { Events } from "@/types/emitter";
import { normalizeString } from "@/utils";
import type { Emitter } from "mitt";
import { storeToRefs } from "pinia";
import { inject, onBeforeMount, onBeforeUnmount } from "vue";
import { inject, onMounted, onBeforeUnmount } from "vue";

// Props
const scanningStore = storeScanning();
Expand All @@ -24,6 +28,8 @@ const emitter = inject<Emitter<Events>>("emitter");
// Props
const heartbeat = storeHeartbeat();
const configStore = storeConfig();
const auth = storeAuth();
const platformsStore = storePlatforms();

socket.on(
"scan:scanning_platform",
Expand Down Expand Up @@ -93,16 +99,44 @@ onBeforeUnmount(() => {
socket.off("scan:scanning_rom");
socket.off("scan:done");
socket.off("scan:done_ko");

document.removeEventListener("network-quiesced", fetchHomeData);
});

onBeforeMount(() => {
api.get("/heartbeat").then(({ data: data }) => {
heartbeat.set(data);
});
onMounted(() => {
api.get("/config").then(({ data: data }) => {
configStore.set(data);
});
});

function fetchHomeData() {
// Remove it so it's not called multiple times
document.removeEventListener("network-quiesced", fetchHomeData);

api.get("/heartbeat").then(({ data: data }) => {
heartbeat.set(data);
});

platformApi
.getPlatforms()
.then(({ data: platforms }) => {
platformsStore.set(platforms);
})
.catch((error) => {
console.error(error);
});

userApi
.fetchCurrentUser()
.then(({ data: user }) => {
auth.setUser(user);
})
.catch((error) => {
console.error(error);
});
}

document.addEventListener("network-quiesced", fetchHomeData);
</script>

<template>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/__generated__/models/PlatformSchema.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion frontend/src/components/Details/Emulation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function onFullScreenChange() {
label="BIOS"
v-model="biosRef"
:items="
props.platform.firmware_files?.map((f) => ({
props.platform.firmware?.map((f) => ({
title: f.file_name,
value: f,
})) ?? []
Expand Down
22 changes: 11 additions & 11 deletions frontend/src/components/Dialog/Platform/ViewFirmware.vue
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function uploadFirmware() {
.then(({ data }) => {
const { uploaded, firmware } = data;
if (selectedPlatform.value) {
selectedPlatform.value.firmware_files = firmware;
selectedPlatform.value.firmware = firmware;
}

emitter?.emit("snackbarShow", {
Expand Down Expand Up @@ -75,8 +75,8 @@ function deleteFirmware() {
.deleteFirmware({ firmware: selectedFirmware.value, deleteFromFs: false })
.then(() => {
if (selectedPlatform.value) {
selectedPlatform.value.firmware_files =
selectedPlatform.value.firmware_files?.filter(
selectedPlatform.value.firmware =
selectedPlatform.value.firmware?.filter(
(firmware) => !selectedFirmware.value.includes(firmware)
);
}
Expand All @@ -91,20 +91,20 @@ function deleteFirmware() {
}

function allFirmwareSelected() {
if (selectedPlatform.value?.firmware_files?.length == 0) {
if (selectedPlatform.value?.firmware?.length == 0) {
return false;
}
return (
selectedFirmware.value.length ===
selectedPlatform.value?.firmware_files?.length
selectedPlatform.value?.firmware?.length
);
}

function selectAllFirmware() {
if (allFirmwareSelected()) {
selectedFirmware.value = [];
} else {
selectedFirmware.value = selectedPlatform.value?.firmware_files ?? [];
selectedFirmware.value = selectedPlatform.value?.firmware ?? [];
}
}
</script>
Expand Down Expand Up @@ -223,14 +223,14 @@ function selectAllFirmware() {
<v-card-text
class="my-4 py-0"
v-if="
selectedPlatform?.firmware_files != undefined &&
selectedPlatform?.firmware_files?.length > 0
selectedPlatform?.firmware != undefined &&
selectedPlatform?.firmware?.length > 0
"
>
<v-list rounded="0" class="pa-0">
<v-list-item
class="px-3"
v-for="firmware in selectedPlatform?.firmware_files ?? []"
v-for="firmware in selectedPlatform?.firmware ?? []"
:key="firmware.id"
>
<template v-slot:prepend>
Expand Down Expand Up @@ -286,8 +286,8 @@ function selectAllFirmware() {
variant="text"
:disabled="
!(
selectedPlatform?.firmware_files != undefined &&
selectedPlatform?.firmware_files?.length > 0
selectedPlatform?.firmware != undefined &&
selectedPlatform?.firmware?.length > 0
)
"
>
Expand Down
10 changes: 0 additions & 10 deletions frontend/src/services/api/filter.ts

This file was deleted.