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

DiagnosticStorage is being allocated eagerly for every IR #10727

Closed
hubertp opened this issue Jul 31, 2024 · 5 comments · Fixed by #10852
Closed

DiagnosticStorage is being allocated eagerly for every IR #10727

hubertp opened this issue Jul 31, 2024 · 5 comments · Fixed by #10852

Comments

@hubertp
Copy link
Collaborator

hubertp commented Jul 31, 2024

For a (very) simple Enso program we seem to allocate a lot of DiagnosticStorage objects:
Screenshot from 2024-07-31 15-49-45

The allocation could be done lazily as typically we need a very small fraction of those.

@hubertp hubertp self-assigned this Aug 6, 2024
@hubertp hubertp moved this from ❓New to 📤 Backlog in Issues Board Aug 6, 2024
@hubertp hubertp assigned 4e6 and unassigned hubertp Aug 6, 2024
@enso-bot
Copy link

enso-bot bot commented Aug 16, 2024

Dmitry Bushev reports a new STANDUP for today (2024-08-16):

Progress: [10727] Started working on the issue. Reproduced a large number of allocations of DiagnosticStorage object in the heap dump. Started working on the lazy diagnostics storage initialization. It should be finished by 2024-08-23.

Next Day: Next day I will be working on the #10727 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Aug 20, 2024

Dmitry Bushev reports a new STANDUP for yesterday (2024-08-19):

Progress: [10727] Implemented lazy diagnostics logic. Implemented lazy diagnostics for all the IR nodes. Started testing It should be finished by 2024-08-23.

Next Day: Next day I will be working on the #10727 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Aug 20, 2024

Dmitry Bushev reports a new STANDUP for today (2024-08-20):

Progress: [10727] Eliminated instantiations of diagnostics in the GatherDiagnostics compiler pass. Eliminate redundant instantiations during the nodes copying in various compiler passes. Fixed the persistence logic for diagnostics storage. Started fixing the tests It should be finished by 2024-08-23.

Next Day: Next day I will be working on the #10727 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Aug 22, 2024

Dmitry Bushev reports a new STANDUP for yesterday (2024-08-21):

Progress: [10727] Fixed the serialization tests. Updated the constructor functions for some IR nodes. Fixed tests for GatherDiagnostics compiler pass. Re-viewed and cleaned up the PR. It should be finished by 2024-08-23.

Next Day: Next day I will be working on the #10727 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Aug 22, 2024

Dmitry Bushev reports a new STANDUP for today (2024-08-22):

Progress: [10727] Refactored the IR interface. Fixing a bunch of merge conflicts. Fixing failing tests on CI. Final clean up and measurements. Undrafted the PR. It should be finished by 2024-08-23.

Next Day: Next day I will be working on the #10727 task. Continue working on the task

@4e6 4e6 moved this from 🔧 Implementation to 👁️ Code review in Issues Board Aug 22, 2024
@mergify mergify bot closed this as completed in #10852 Aug 23, 2024
mergify bot pushed a commit that referenced this issue Aug 23, 2024
close #10727



Changelog:
- add: implement lazy allocation of `DiagnosticStorage` objects
- update: compiler phases to make sure they don't allocate redundant `DiagnosticStorage` objects
- update: cleanup `DiagnosticStorage` class

# Important Notes
As a result, all 10MB of redundant `DiagnosticStorage` allocations are gone

#### Before
![2024-08-21-170148_2092x171_scrot](https://github.com/user-attachments/assets/c1fd34d5-019d-472f-b523-a5c31b87f454)

#### After
![2024-08-21-170058_1693x131_scrot](https://github.com/user-attachments/assets/10d71d81-42b7-4b3c-a49f-ca6267bc6ccf)
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants