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.

Sign up for GitHub

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

Bundle Analysis: Add asset comparison and fetching PR files #480

Merged
merged 4 commits into from
Jan 22, 2025
Merged

Bundle Analysis: Add asset comparison and fetching PR files #480

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb9a1ba
initial commit
JerrySentry Jan 14, 2025
eee4506
add unit tests
JerrySentry Jan 16, 2025
c170004
add in match by uuid and tests
JerrySentry Jan 16, 2025
9be52d8
Merge branch 'main' into jan_14_large_files
JerrySentry Jan 20, 2025
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
85 changes: 61 additions & 24 deletions shared/bundle_analysis/comparison.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
BundleAnalysisReport,
BundleReport,
BundleRouteReport,
ModuleReport,
)
from shared.bundle_analysis.storage import BundleAnalysisReportLoader
from shared.django_apps.core.models import Repository
Expand Down Expand Up @@ -77,11 +78,66 @@ class AssetChange(BaseChange):
"""

asset_name: str
percentage_delta: float
size_base: int
size_head: int


AssetMatch = Tuple[Optional[AssetReport], Optional[AssetReport]]


class AssetComparison:
def __init__(
self,
base_asset_report: Optional[AssetReport] = None,
head_asset_report: Optional[AssetReport] = None,
):
self.base_asset_report = base_asset_report
self.head_asset_report = head_asset_report

@sentry_sdk.trace
def asset_change(self) -> AssetChange:
if self.base_asset_report is None:
return AssetChange(
asset_name=self.head_asset_report.name,
change_type=AssetChange.ChangeType.ADDED,
size_delta=self.head_asset_report.size,
percentage_delta=100,
size_base=0,
size_head=self.head_asset_report.size,
)
elif self.head_asset_report is None:
return AssetChange(
asset_name=self.base_asset_report.name,
change_type=AssetChange.ChangeType.REMOVED,
size_delta=-self.base_asset_report.size,
percentage_delta=-100.0,
size_base=self.base_asset_report.size,
size_head=0,
)
else:
size_delta = self.head_asset_report.size - self.base_asset_report.size
percentage_delta = round(
(size_delta / self.base_asset_report.size) * 100, 2
)
return AssetChange(
asset_name=self.head_asset_report.name,
change_type=AssetChange.ChangeType.CHANGED,
size_delta=size_delta,
percentage_delta=percentage_delta,
size_base=self.base_asset_report.size,
size_head=self.head_asset_report.size,
)

def contributing_modules(
self, pr_changed_files: Optional[List[str]] = None
) -> List[ModuleReport]:
asset_report = self.head_asset_report
if asset_report is None:
return []
return asset_report.modules(pr_changed_files)


class BundleComparison:
def __init__(
self, base_bundle_report: BundleReport, head_bundle_report: BundleReport
Expand All @@ -95,7 +151,7 @@ def total_size_delta(self) -> int:
return head_size - base_size

@sentry_sdk.trace
def asset_changes(self) -> List[AssetChange]:
def asset_comparisons(self) -> List[AssetComparison]:
# this groups assets by name
# there can be multiple assets with the same name and we
# need to try and match them across base and head reports
Expand All @@ -119,29 +175,10 @@ def asset_changes(self) -> List[AssetChange]:
if asset_name not in asset_names:
matches += self._match_assets(asset_reports, [])

changes = []
for base_asset_report, head_asset_report in matches:
if base_asset_report is None:
change = AssetChange(
asset_name=head_asset_report.name,
change_type=AssetChange.ChangeType.ADDED,
size_delta=head_asset_report.size,
)
elif head_asset_report is None:
change = AssetChange(
asset_name=base_asset_report.name,
change_type=AssetChange.ChangeType.REMOVED,
size_delta=-base_asset_report.size,
)
else:
change = AssetChange(
asset_name=head_asset_report.name,
change_type=AssetChange.ChangeType.CHANGED,
size_delta=head_asset_report.size - base_asset_report.size,
)
changes.append(change)

return changes
return [
AssetComparison(base_asset_report, head_asset_report)
for base_asset_report, head_asset_report in matches
]

def _match_assets(
self,
Expand Down
11 changes: 11 additions & 0 deletions shared/bundle_analysis/parsers/v3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import ijson
import sentry_sdk
from sqlalchemy import tuple_
from sqlalchemy.orm import Session as DbSession
from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound

Expand Down Expand Up @@ -160,6 +161,16 @@ def parse(self, path: str) -> Tuple[int, str]:
# but first we need to find the Asset by the hashed file name
dynamic_imports_list = self._parse_dynamic_imports()
if dynamic_imports_list:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delete the old entries in the table before inserting the new data.

self.db_session.execute(
DynamicImport.__table__.delete().where(
tuple_(DynamicImport.chunk_id, DynamicImport.asset_id).in_(
[
(item["chunk_id"], item["asset_id"])
for item in dynamic_imports_list
]
)
)
)
insert_dynamic_imports = DynamicImport.__table__.insert().values(
dynamic_imports_list
)
Expand Down
20 changes: 16 additions & 4 deletions shared/bundle_analysis/report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,28 @@ def uuid(self) -> str:
def asset_type(self) -> AssetType:
return self.asset.asset_type

def modules(self) -> List[ModuleReport]:
def modules(
self, pr_changed_files: Optional[List[str]] = None
) -> List[ModuleReport]:
with get_db_session(self.db_path) as session:
modules = (
query = (
session.query(Module)
.join(Module.chunks)
.join(Chunk.assets)
.filter(Asset.id == self.asset.id)
.all()
)
return [ModuleReport(self.db_path, module) for module in modules]

# Apply additional filter if pr_changed_files is provided
if pr_changed_files:
# Normalize pr_changed_files to have './' prefix for relative paths
normalized_files = {
f"./{file}" if not file.startswith("./") else file
for file in pr_changed_files
}
query = query.filter(Module.name.in_(normalized_files))

modules = query.all()
return [ModuleReport(self.db_path, module) for module in modules]

def routes(self) -> Optional[List[str]]:
plugin_name = self.bundle_info.get("plugin_name")
Expand Down
189 changes: 154 additions & 35 deletions tests/unit/bundle_analysis/test_bundle_comparison.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,45 +111,164 @@ def test_bundle_analysis_comparison():
)

bundle_comparison = comparison.bundle_comparison("sample")
asset_changes = bundle_comparison.asset_changes()
assert set(asset_changes) == set(
[
AssetChange(
asset_name="assets/other-*.svg",
change_type=AssetChange.ChangeType.ADDED,
size_delta=5126,
),
AssetChange(
asset_name="assets/index-*.css",
change_type=AssetChange.ChangeType.CHANGED,
size_delta=0,
),
AssetChange(
asset_name="assets/LazyComponent-*.js",
change_type=AssetChange.ChangeType.CHANGED,
size_delta=0,
),
AssetChange(
asset_name="assets/index-*.js",
change_type=AssetChange.ChangeType.CHANGED,
size_delta=100,
),
AssetChange(
asset_name="assets/index-*.js",
change_type=AssetChange.ChangeType.CHANGED,
size_delta=0,
),
AssetChange(
asset_name="assets/react-*.svg",
change_type=AssetChange.ChangeType.REMOVED,
size_delta=-4126,
),
]
asset_comparisons = bundle_comparison.asset_comparisons()
assert len(asset_comparisons) == 6

asset_comparison_d = {}
for asset_comparison in asset_comparisons:
key = (
asset_comparison.base_asset_report.hashed_name
if asset_comparison.base_asset_report
else None,
asset_comparison.head_asset_report.hashed_name
if asset_comparison.head_asset_report
else None,
)
assert key not in asset_comparison_d
asset_comparison_d[key] = asset_comparison

# Check asset change is correct
assert asset_comparison_d[
("assets/index-666d2e09.js", "assets/index-666d2e09.js")
].asset_change() == AssetChange(
change_type=AssetChange.ChangeType.CHANGED,
size_delta=0,
asset_name="assets/index-*.js",
percentage_delta=0,
size_base=144577,
size_head=144577,
)
assert asset_comparison_d[
("assets/index-c8676264.js", "assets/index-c8676264.js")
].asset_change() == AssetChange(
change_type=AssetChange.ChangeType.CHANGED,
size_delta=100,
asset_name="assets/index-*.js",
percentage_delta=64.94,
size_base=154,
size_head=254,
)
assert asset_comparison_d[
(None, "assets/other-35ef61ed.svg")
].asset_change() == AssetChange(
change_type=AssetChange.ChangeType.ADDED,
size_delta=5126,
asset_name="assets/other-*.svg",
percentage_delta=100,
size_base=0,
size_head=5126,
)
assert asset_comparison_d[
("assets/index-d526a0c5.css", "assets/index-d526a0c5.css")
].asset_change() == AssetChange(
change_type=AssetChange.ChangeType.CHANGED,
size_delta=0,
asset_name="assets/index-*.css",
percentage_delta=0,
size_base=1421,
size_head=1421,
)
assert asset_comparison_d[
("assets/LazyComponent-fcbb0922.js", "assets/LazyComponent-fcbb0922.js")
].asset_change() == AssetChange(
change_type=AssetChange.ChangeType.CHANGED,
size_delta=0,
asset_name="assets/LazyComponent-*.js",
percentage_delta=0,
size_base=294,
size_head=294,
)
assert asset_comparison_d[
("assets/react-35ef61ed.svg", None)
].asset_change() == AssetChange(
change_type=AssetChange.ChangeType.REMOVED,
size_delta=-4126,
asset_name="assets/react-*.svg",
percentage_delta=-100,
size_base=4126,
size_head=0,
)

# Check asset contributing modules is correct
module_reports = asset_comparison_d[
("assets/index-666d2e09.js", "assets/index-666d2e09.js")
].contributing_modules()
assert [module.name for module in module_reports] == [
"./vite/modulepreload-polyfill",
"./commonjsHelpers.js",
"../../node_modules/.pnpm/[email protected]/node_modules/react/jsx-runtime.js?commonjs-module",
"../../node_modules/.pnpm/[email protected]/node_modules/react/cjs/react-jsx-runtime.production.min.js?commonjs-exports",
"../../node_modules/.pnpm/[email protected]/node_modules/react/index.js?commonjs-module",
"../../node_modules/.pnpm/[email protected]/node_modules/react/cjs/react.production.min.js?commonjs-exports",
"../../node_modules/.pnpm/[email protected]/node_modules/react/cjs/react.production.min.js",
"../../node_modules/.pnpm/[email protected]/node_modules/react/index.js",
"../../node_modules/.pnpm/[email protected]/node_modules/react/cjs/react-jsx-runtime.production.min.js",
"../../node_modules/.pnpm/[email protected]/node_modules/react/jsx-runtime.js",
"../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/client.js?commonjs-exports",
"../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/index.js?commonjs-module",
"../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js?commonjs-exports",
"../../node_modules/.pnpm/[email protected]/node_modules/scheduler/index.js?commonjs-module",
"../../node_modules/.pnpm/[email protected]/node_modules/scheduler/cjs/scheduler.production.min.js?commonjs-exports",
"../../node_modules/.pnpm/[email protected]/node_modules/scheduler/cjs/scheduler.production.min.js",
"../../node_modules/.pnpm/[email protected]/node_modules/scheduler/index.js",
"../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js",
"../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/index.js",
"../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/client.js",
"./vite/preload-helper",
"./src/assets/react.svg",
"../../../../../../vite.svg",
"./src/App.css",
"./src/App.tsx",
"./src/index.css",
"./src/main.tsx",
"./index.html",
]
module_reports = asset_comparison_d[
("assets/index-c8676264.js", "assets/index-c8676264.js")
].contributing_modules()
assert [module.name for module in module_reports] == [
"./src/IndexedLazyComponent/IndexedLazyComponent.tsx",
"./src/IndexedLazyComponent/index.ts",
"./src/Other.tsx",
]
module_reports = asset_comparison_d[
(None, "assets/other-35ef61ed.svg")
].contributing_modules()
assert [module.name for module in module_reports] == []
module_reports = asset_comparison_d[
("assets/index-d526a0c5.css", "assets/index-d526a0c5.css")
].contributing_modules()
assert [module.name for module in module_reports] == []
module_reports = asset_comparison_d[
("assets/LazyComponent-fcbb0922.js", "assets/LazyComponent-fcbb0922.js")
].contributing_modules()
assert [module.name for module in module_reports] == [
"./src/LazyComponent/LazyComponent.tsx",
]
module_reports = asset_comparison_d[
("assets/react-35ef61ed.svg", None)
].contributing_modules()
assert [module.name for module in module_reports] == []

# Check asset contributing modules is correct when pr_files are used for filtering
module_reports = asset_comparison_d[
("assets/index-666d2e09.js", "assets/index-666d2e09.js")
].contributing_modules(
pr_changed_files=[
"src/index.css",
"src/main.tsx",
"./index.html",
"/src/App.css",
] # first 3 is in, 4th is not
)
assert [module.name for module in module_reports] == [
"./src/index.css",
"./src/main.tsx",
"./index.html",
]

total_size_delta = bundle_comparison.total_size_delta()
assert total_size_delta == 1100
assert total_size_delta == sum([change.size_delta for change in asset_changes])
assert comparison.percentage_delta == 0.73

with pytest.raises(MissingBundleError):
Expand Down
Loading