Skip to content

Conversation

@kbirken
Copy link
Member

@kbirken kbirken commented Dec 1, 2025

The intention "Update all configurations" for feature models has been improved significantly:

  • Now a modal dialog with a progress bar and some additional information is shown.
  • The process is now completely sequential, improving stability.
  • Moreover, there is some timing output in the log.
  • Finally, performance has been improved by postponing the event handling for all changes to the involved models.

Example of the new progress bar dialog:

image

The CHANGELOG has been updated (and some empty lines have been added to unify the formatting).

In this PR we did not change the actual solver task generation. Thus, for big feature models, the solver task generation for a couple of configurations now is the part with most computation effort (after the other optimisations have been done, like event postponing). However, changing the solver generator structure would induce a lot of implementation effort and risk, so skipping it for now.

This solves #1530.

@kbirken kbirken self-assigned this Dec 1, 2025
@kbirken kbirken requested a review from mfsoliveira December 1, 2025 17:30
mfsoliveira
mfsoliveira previously approved these changes Dec 2, 2025
Copy link
Member

@mfsoliveira mfsoliveira left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements.
Besides that I could see how to implement the progress with model dialog. My implementation for pv importer didn't work well.

Questions:

  1. On Cancel: Could be that the write step should be protected, so that it happens atomic? Or is it already so? Think the cancel in the middle of the write could create a model that can no longer be updated, and it is in inconsistent state. But I also think we can call it manually any way later. However the user must find out it.

Suggestions:

  1. Conditional call depending on boolean parameter: I see a lot of this construct. Sometimes it is in the middle of some complex algorithm, and the easiest way if just add the parameter for the condition. But when the parameter is added to improve reuse, like in ConfigUpdateHelper.runSolverIfNotIgnored., what we really want to reuse it the test condition. We could extract the condition "isSolverIgnored" and call direct
    runSolverIfNotIgnoredAsync or runSolverIfNotIgnoredSync without the boolean parameter.

Details:
Implement model dialog with progress bar for syncronous update of all configs.
Split each configuration update into two steps, postpone model updates and add timing measurements.
Enforce update of editors of updated configurations.
Refactor code based on review suggestion.
Update CHANGELOG and unify formatting.
@kbirken kbirken force-pushed the feature/improve_update_all_configs_intention_1530 branch from e471b0d to 9ed89c4 Compare December 2, 2025 16:24
@kbirken
Copy link
Member Author

kbirken commented Dec 2, 2025

Thanks for the review. Regarding your question about cancel: The updates of each configuration is atomic, but the whole loop is not. I.e., if the user presses cancel, then all configurations up to the currently updated one will be actually updated, the succeeding configurations will not be updated. I guess it would be too much effort right now to make the whole process atomic. And as you mentioned before, the user can always re-run the intention and update the remaining configs.

Regarding your suggestion: I refactored the code accordingly and force-pushed the changes in a squashed commit.

@kbirken kbirken enabled auto-merge December 2, 2025 16:28
…re/improve_update_all_configs_intention_1530

# Conflicts:
#	CHANGELOG.md
@mfsoliveira mfsoliveira self-requested a review December 3, 2025 08:28
Copy link
Member

@mfsoliveira mfsoliveira left a comment

Choose a reason for hiding this comment

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

Thanks for the answer and changes.

@kbirken kbirken merged commit def34f8 into maintenance/mps20241 Dec 3, 2025
2 checks passed
@kbirken kbirken deleted the feature/improve_update_all_configs_intention_1530 branch December 3, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants