[rocpd] Schema Updates#347
Conversation
c303613 to
7ddd5da
Compare
7ddd5da to
c235cff
Compare
c235cff to
f5335e5
Compare
bwelton
left a comment
There was a problem hiding this comment.
This may have been on the original PR, but can you add some context about why this is being changed and what the effect of the change is?
| for name in tbls: | ||
| log(f"Inserting rows into {name} from {alias}.{name}") | ||
| conn.execute(f'INSERT INTO "{name}" SELECT * FROM {alias}."{name}"') |
There was a problem hiding this comment.
| for name in tbls: | |
| log(f"Inserting rows into {name} from {alias}.{name}") | |
| conn.execute(f'INSERT INTO "{name}" SELECT * FROM {alias}."{name}"') | |
| for name in tbls: | |
| log(f"Inserting rows into {name} from {alias}.{name}") | |
| rows = conn.execute(f'SELECT * FROM {alias}."{name}"').fetchall() | |
| if rows: | |
| col_count = len(rows[0]) | |
| placeholders = ", ".join(["?"] * col_count) | |
| conn.executemany( | |
| f'INSERT INTO "{name}" VALUES ({placeholders})', | |
| rows | |
| ) |
Looking at merge.py, @bgopesh suggested I change this to a batch operation in my PR. Doing that in your implementation will halve the merge time! I'm looking at ways of incorporating your merge.py with the merge.py that the Silo team wrote in my PR. But this is a good change to pickup.
| # Optional: enforce integrity | ||
| try: | ||
| conn.execute("PRAGMA quick_check;") | ||
| except sqlite3.DatabaseError as e: | ||
| log(f"SQLite3 quick_check reported an issue: {e}") |
There was a problem hiding this comment.
This integrity check (I see you have it marked as optional here in the comments) will add a 66%-175% time hit to merge time. It may be better to skip it, or have it as a user-opt-in check (but what user will really want to turn this on?).
ie, merging 2 13MB DBs, merge time goes from 3.67 seconds to 1.33 seconds when we skip this check.
merging 16 13MB DBs, merge time goes from 37.03 seconds to 22.23 second when we skip this check.
be09959 to
f1b9797
Compare
- support multi-process stable folder naming convention
- add merge submodule - combine multiple databases into one - add functions submodule - defines (in Python) such as STDDEV_SAMP - add/update bindings submodule - support using rocpd without Python bindings - not all features are supported in pure python yet
- original schema is v3.0.0 - new schema is v4.0.0
- all python classes are CamelCase - fully implement bindings.py - fix issues with counter collection in csv.py - use lds_block_size instead of lds_size - support using rocpd in RocpdReader - handle .rpdb files in attachment tests - cleanup .rpdb files generated in tests - remove generation of database in tests/rocprofv3/counter-collection/list_metrics - fix test_csv_data to not ignore roctxMarkA
b3840d6 to
da074da
Compare
* Change to enable ibgda bitcode compilation * Apply suggestion from @abouteiller --------- Co-authored-by: Aurelien Bouteiller <aurelien.bouteiller@amd.com>
* Change to enable ibgda bitcode compilation * Apply suggestion from @abouteiller --------- Co-authored-by: Aurelien Bouteiller <aurelien.bouteiller@amd.com> [ROCm/rocshmem commit: fbe5730]
|
This pull request has been inactive for 25 days and will be marked as stale. If you would like to keep this PR open, please:
This PR will be automatically closed in 5 days if no further activity occurs. |
|
Do NOT close yet, we are still referencing this PR to move the necessary pieces into develop. |
|
This pull request has been inactive for 25 days and will be marked as stale. If you would like to keep this PR open, please:
This PR will be automatically closed in 5 days if no further activity occurs. |
|
BUMP, still referencing this PR for schema updates. |
PR Details
Associated Jira Ticket Number/Link
What type of PR is this? (check all applicable)
Technical details
Added/updated tests?
Updated CHANGELOG?
Added/Updated documentation?
🔁 Imported from ROCm/rocprofiler-sdk#118
🧑💻 Originally authored by @rocm-devops