Skip to content

Commit

Permalink
Streamline bloat reports, part 2 of 2 (#33716)
Browse files Browse the repository at this point in the history
* Streamline bloat reports, part 2

Remove the early attempt to aggregate FLASH vs RAM based on whether
ELF segments are writable or not, which turned out not to be useful
because several platforms mark flash segments as writable. Now that
explicit section groups are in use (#33642), there is no additional
value to segment aggregation even on platforms that mark them
accurately.

* Streamline bloat reports, part 2

Remove the early attempt to aggregate FLASH vs RAM based on whether
ELF segments are writable or not, which turned out not to be useful
because several platforms mark flash segments as writable. Now that
explicit section groups are in use (#33642), there is no additional
value to segment aggregation even on platforms that mark them
accurately.

* Filter down to region reports only.
  • Loading branch information
kpschoedel authored Jun 4, 2024
1 parent 2feb25f commit a63cf02
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 34 deletions.
4 changes: 4 additions & 0 deletions scripts/tools/memory/gh_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,10 @@ def report_matching_commits(self) -> Dict[str, pd.DataFrame]:
continue

df = pd.DataFrame(changes.rows, columns=changes.columns)

# Filter down to region reports only.
df = df[df['kind'] == 'region'].drop('kind', axis=1)

df.attrs = {
'name': f'{pr},{parent},{commit}',
'title': (f'PR #{pr}: ' if pr else '') +
Expand Down
35 changes: 10 additions & 25 deletions scripts/tools/memory/gh_sizes.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@
{"section": ".data", "size": 1648},
{"section": ".text", "size": 740236}
],
"wr": [
{"wr": 0, "size": 262144},
{"wr": 1, "size": 74023}
"region": [
{"region": "FLASH", "size": 262144},
{"region": "RAM", "size": 74023}
]
}
}
Expand All @@ -77,8 +77,7 @@
import memdf.report
import memdf.select
import memdf.util
import numpy as np # type: ignore
from memdf import Config, ConfigDescription, DFs, SectionDF, SegmentDF
from memdf import Config, ConfigDescription, DFs, SectionDF

PLATFORM_CONFIG_DIR = pathlib.Path('scripts/tools/memory/platform')

Expand Down Expand Up @@ -162,7 +161,8 @@ def main(argv):
**CONFIG,
}
# In case there is no platform configuration file, default to using a popular set of section names.
config_desc['section.select']['default'] = ['.text', '.rodata', '.data', '.bss']
config_desc['section.select']['default'] = [
'.text', '.rodata', '.data', '.bss']

config = Config().init(config_desc)
config.put('output.file', output)
Expand Down Expand Up @@ -197,32 +197,17 @@ def main(argv):

collected: DFs = memdf.collect.collect_files(config, [binary])

# Aggregate loaded segments, by writable (RAM) or not (flash).
segments = collected[SegmentDF.name]
segments['segment'] = segments.index
segments['wr'] = ((segments['flags'] & 2) != 0).convert_dtypes(
convert_boolean=False, convert_integer=True)
segment_summary = segments[segments['type'] == 'PT_LOAD'][[
'wr', 'size'
]].groupby('wr').aggregate(np.sum).reset_index().astype(
{'size': np.int64})
segment_summary.attrs['name'] = "wr"

sections = collected[SectionDF.name]
sections = sections.join(on='segment',
how='left',
other=segments,
rsuffix='-segment')
section_summary = sections[['section', 'size',
'wr']].sort_values(by='section')
section_summary = sections[['section',
'size']].sort_values(by='section')
section_summary.attrs['name'] = "section"

region_summary = memdf.select.groupby(config, collected['section'], 'region')
region_summary = memdf.select.groupby(
config, collected['section'], 'region')
region_summary.attrs['name'] = "region"

summaries = {
'section': section_summary,
'memory': segment_summary,
'region': region_summary,
}

Expand Down
12 changes: 3 additions & 9 deletions scripts/tools/memory/memdf/sizedb.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,6 @@ def add_sizes_from_json(self, s: Union[bytes, str], origin: Dict):
'size': i['size'],
'kind': frame
})
# Add segment sizes.
for i in r['frames'].get('wr', []):
r['sizes'].append({
'name': ('(read only)', '(read/write)')[int(i['wr'])],
'size': i['size'],
'kind': 'wr'
})
self.add_sizes(**r)

def add_sizes_from_zipfile(self, f: Union[IO, Path], origin: Dict):
Expand Down Expand Up @@ -182,6 +175,7 @@ def select_changes(self, parent: str, commit: str) -> ChangeInfo:
pb.id AS parent_build,
cb.id AS commit_build,
t.platform, t.config, t.target,
cs.kind AS kind,
cs.name AS name,
ps.size AS parent_size,
cs.size AS commit_size,
Expand All @@ -196,7 +190,7 @@ def select_changes(self, parent: str, commit: str) -> ChangeInfo:
cs.name, cb.time DESC, pb.time DESC
''', (commit, parent))

keep = ('platform', 'target', 'config', 'name', 'parent_size',
keep = ('platform', 'target', 'config', 'kind', 'name', 'parent_size',
'commit_size')
things: set[int] = set()
artifacts: set[int] = set()
Expand Down Expand Up @@ -229,7 +223,7 @@ def select_changes(self, parent: str, commit: str) -> ChangeInfo:
artifacts.add(row['artifact'])
builds.add(row['commit_build'])

return ChangeInfo(('platform', 'target', 'config', 'section',
return ChangeInfo(('platform', 'target', 'config', 'kind', 'section',
parent[:8], commit[:8], 'change', '% change'), rows,
things, builds, stale_builds, artifacts,
stale_artifacts)
Expand Down

0 comments on commit a63cf02

Please sign in to comment.