Skip to content

Introduce IceTipRepositoryStatusModel to avoid multiple commit lookups#1918

Merged
jecisc merged 2 commits intopharo-vcs:Pharo14from
guillep:fix-commit-lookups
Dec 7, 2025
Merged

Introduce IceTipRepositoryStatusModel to avoid multiple commit lookups#1918
jecisc merged 2 commits intopharo-vcs:Pharo14from
guillep:fix-commit-lookups

Conversation

@guillep
Copy link
Member

@guillep guillep commented Mar 28, 2025

Opening iceberg does a lot of commit lookups to show info on the browsers.
This in turn stresses a lot the GC and the ephemeron queue because all those commits are set as autorelease.

This change adds a description object where values can be cached per table.
This allows to go from 1111 to ~33 lookups in my testing environment.

@Ducasse
Copy link
Collaborator

Ducasse commented Jun 29, 2025

@guillep what should we do? I can integrate this fix.

@jecisc jecisc closed this Sep 29, 2025
@jecisc jecisc reopened this Sep 29, 2025
@jecisc
Copy link
Contributor

jecisc commented Oct 1, 2025

Some tests are failing but I think they are just failing in P13 branch and got fixed in P14.

Should we integrate and port forward to P14?

@jecisc jecisc changed the base branch from Pharo13 to Pharo14 December 7, 2025 02:44
@Ducasse
Copy link
Collaborator

Ducasse commented Dec 7, 2025

@guillep @jecisc I would integrate this in P14. Is it ok?

@jecisc
Copy link
Contributor

jecisc commented Dec 7, 2025

Yes let's merge.
I was waiting to see if we could have another review because the Iceberg model is still a little obscure to me. But it would be a shame to lose an improvement like this :)

@jecisc jecisc merged commit ec52990 into pharo-vcs:Pharo14 Dec 7, 2025
0 of 3 checks passed
@jecisc
Copy link
Contributor

jecisc commented Dec 7, 2025

I have the impression that this broke a few things in Iceberg but I don't understand what is happening for now. I'll probably check this week

@jecisc
Copy link
Contributor

jecisc commented Dec 7, 2025

I found the error but I need to discuss with Esteban and Guille because I think this is a Spec bug but we need to decide what is the right behavior

@jecisc
Copy link
Contributor

jecisc commented Dec 7, 2025

I will revert this change for now because even with a fix in Spec, this is breaking the coloration in the repositories list

jecisc added a commit that referenced this pull request Dec 7, 2025
This reverts commit ec52990, reversing
changes made to bb315df.
@jecisc
Copy link
Contributor

jecisc commented Dec 7, 2025

Reverted in: 688b39d

jecisc added a commit to jecisc/iceberg that referenced this pull request Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants