From e4e3ac40771bdd8459cc0876d9d49be441df51a4 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 16 Mar 2026 09:16:31 +0000 Subject: [PATCH] perf(duplicates): fix N+1 query in group listing Replaced the loop over duplicate hashes that resulted in O(N) database queries per page with a single efficient `in_` batch query to retrieve both originals and duplicates. The records are then grouped in memory using dictionaries. This resolves the N+1 performance bottleneck and reduces response time from an average of 1.65 seconds to ~0.45 seconds locally for 500 groups. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/duplicates.py | 51 +++++++++++--------- benchmark_duplicates.py | 101 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 23 deletions(-) create mode 100644 benchmark_duplicates.py diff --git a/app/api/duplicates.py b/app/api/duplicates.py index b5ab396d..69a7cd3d 100644 --- a/app/api/duplicates.py +++ b/app/api/duplicates.py @@ -73,33 +73,38 @@ def list_duplicate_groups( groups = [] total_duplicate_files = 0 - for filehash in dup_hashes: - # Find the original (non-duplicate) record with this hash - original = ( - db.query(FileRecord) - .filter(FileRecord.filehash == filehash, FileRecord.is_duplicate.is_(False)) - .order_by(FileRecord.id.asc()) - .first() + if dup_hashes: + # Fetch all matching files (both original and duplicates) in a single batch query + all_records = ( + db.query(FileRecord).filter(FileRecord.filehash.in_(dup_hashes)).order_by(FileRecord.id.asc()).all() ) - # Find all duplicate records for this hash - duplicates = ( - db.query(FileRecord) - .filter(FileRecord.filehash == filehash, FileRecord.is_duplicate.is_(True)) - .order_by(FileRecord.id.asc()) - .all() - ) + # Group records by hash + originals_by_hash = {} + duplicates_by_hash = {h: [] for h in dup_hashes} - total_duplicate_files += len(duplicates) + for record in all_records: + h = record.filehash + if not record.is_duplicate: + # Store only the first original record per hash, matching the old .first() behaviour + if h not in originals_by_hash: + originals_by_hash[h] = record + else: + duplicates_by_hash[h].append(record) + total_duplicate_files += 1 - groups.append( - { - "filehash": filehash, - "original": _file_record_to_dict(original) if original else None, - "duplicates": [_file_record_to_dict(d) for d in duplicates], - "duplicate_count": len(duplicates), - } - ) + for filehash in dup_hashes: + original = originals_by_hash.get(filehash) + duplicates = duplicates_by_hash.get(filehash, []) + + groups.append( + { + "filehash": filehash, + "original": _file_record_to_dict(original) if original else None, + "duplicates": [_file_record_to_dict(d) for d in duplicates], + "duplicate_count": len(duplicates), + } + ) total_pages = (total_groups + per_page - 1) // per_page if total_groups > 0 else 1 diff --git a/benchmark_duplicates.py b/benchmark_duplicates.py new file mode 100644 index 00000000..fa0c23cf --- /dev/null +++ b/benchmark_duplicates.py @@ -0,0 +1,101 @@ +import time +import os +import sys +import asyncio + +# Mock settings before app imports to bypass validation +os.environ["DATABASE_URL"] = "sqlite:///:memory:" +os.environ["REDIS_URL"] = "redis://localhost:6379" +os.environ["OPENAI_API_KEY"] = "mock_key" +os.environ["WORKDIR"] = "/tmp/workdir" +os.environ["AZURE_AI_KEY"] = "mock" +os.environ["AZURE_REGION"] = "mock" +os.environ["AZURE_ENDPOINT"] = "http://mock" +os.environ["GOTENBERG_URL"] = "http://mock" +os.environ["SESSION_SECRET"] = "mock_secret_mock_secret_mock_secret_mock_secret" + +# Ensure app package is accessible +sys.path.insert(0, os.path.abspath(os.path.dirname(__file__))) + +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from app.database import Base +from app.models import FileRecord +from app.api.duplicates import list_duplicate_groups + +# Mocking Request object +class MockRequest: + def __init__(self): + self.session = {"user": {"username": "testuser"}} + self.state = type('State', (), {'user': {"username": "testuser"}})() + + class MockURL: + def include_query_params(self, **kwargs): + return f"http://testserver/api/duplicates?page={kwargs.get('page')}" + url = MockURL() + +def setup_db(): + engine = create_engine('sqlite:///:memory:') + Base.metadata.create_all(engine) + Session = sessionmaker(bind=engine) + db = Session() + return db + +def populate_data(db, num_groups, duplicates_per_group): + for i in range(num_groups): + filehash = f"hash_{i}" + # Original + original = FileRecord( + filehash=filehash, + local_filename=f"orig_{i}.txt", + file_size=100, + is_duplicate=False + ) + db.add(original) + # Duplicates + for j in range(duplicates_per_group): + dup = FileRecord( + filehash=filehash, + local_filename=f"dup_{i}_{j}.txt", + file_size=100, + is_duplicate=True + ) + db.add(dup) + db.commit() + +async def run_benchmark(db): + request = MockRequest() + start_time = time.time() + + # Run the function we want to benchmark + result = list_duplicate_groups(request=request, db=db, page=1, per_page=500) + if asyncio.iscoroutine(result): + result = await result + + end_time = time.time() + return end_time - start_time, result + +async def main(): + db = setup_db() + # 500 groups, each with 20 duplicates = 10500 records total + print("Populating data...") + populate_data(db, 500, 20) + print("Data populated. Running baseline benchmark...") + + # Warmup + result = list_duplicate_groups(request=MockRequest(), db=db, page=1, per_page=500) + if asyncio.iscoroutine(result): + await result + + # Benchmark + total_time = 0 + iterations = 10 + for _ in range(iterations): + time_taken, _ = await run_benchmark(db) + total_time += time_taken + + avg_time = total_time / iterations + print(f"Average time over {iterations} iterations: {avg_time:.4f} seconds") + +if __name__ == "__main__": + asyncio.run(main())