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

Jump to bottom

Potential build incrementality issue #293

Open
Youssef1313 opened this issue Jun 10, 2024 · 6 comments · May be fixed by #294
Open

Potential build incrementality issue #293

Youssef1313 opened this issue Jun 10, 2024 · 6 comments · May be fixed by #294
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/enhancement New feature or request. kind/performance Categorizes issue or PR as related to improving performance (allocations/memory/init/etc)

Comments

@Youssef1313
Copy link
Member

The target GenerateUnoSplashAndroid doesn't specify inputs and outputs, and it writes files to disk but doesn't do <FileWrites Include="..." />. This looks like a problem as clean may not delete such files, and also the target will be run on every build even if no changes are done. This will then cause other targets to also be run because new files are written, which may cascade to even run R8 and Csc unnecessarily which are expensive tasks to run on every build.
@Youssef1313 Youssef1313 added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Jun 10, 2024
@jeromelaban
Copy link
Member

Do you know which files are not in FileWrites? I thought we added them all.

@Youssef1313
Copy link
Member Author

Double checked, they are in FileWrites, my bad. However, the lack of Inputs / Outputs make the target run unnecessarily and significantly slowing down incremental builds

@Youssef1313 Youssef1313 linked a pull request Jun 10, 2024 that will close this issue
Open
7 tasks
@Youssef1313
Copy link
Member Author

Uno core is using a very old version of resizetizer actually. I'll update it to confirm that this is still an issue, then will verify if #294 fixes it.

@Youssef1313
Copy link
Member Author

Updating to latest didn't help. However, #294 is helping.

With #294:

image

Without it was always executed, which makes sense because the target doesn't define any inputs.

@dansiegel
Copy link
Contributor

I'm putting this one in the icebox. This is a known issue. This will need to be addressed as part of an overall overhaul of Resizetizer. There are number of paper cuts like this that will need to be addressed.

@dansiegel dansiegel added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/enhancement New feature or request. kind/performance Categorizes issue or PR as related to improving performance (allocations/memory/init/etc) and removed kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Jun 10, 2024
@Youssef1313
Copy link
Member Author

@dansiegel I think it should be a small fix like what I did in #294?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/enhancement New feature or request. kind/performance Categorizes issue or PR as related to improving performance (allocations/memory/init/etc)
Projects
None yet
Milestone
No milestone
3 participants
@dansiegel @jeromelaban @Youssef1313