haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.61k stars 351 forks source link

Fix resultBuilt(dirty mechanism) in hls-graph #4238

Closed soulomoon closed 3 weeks ago

soulomoon commented 1 month ago

Fix #4237 resultBuilt might updated to an older value, which make a rule result that should be clean back to dirty. It might trigger un-needed recomputation. Fixing it could minimize the recomputation further.

michaelpj commented 1 month ago

Do we have any tests for this mechanism? hls-graph is reasonably stand-alone, I would hope we could write a test for this?

wz1000 commented 1 month ago

this seems to make sense. how did you discover this?

soulomoon commented 1 month ago

Actually, I've discovered the Changed and Built are somewhat off when fixing the dritiness hiding problem, but I forget about it at the time since it it won't hide the dirtiness but instead increase the chance of it. I am reviewing the code and it comes back to me.

Yep, I'll see if some test could be added.

soulomoon commented 1 month ago

I've added two test, one from the lower level and one from higher level to demonstrate the bug and ensure no regression.

michaelpj commented 1 month ago

I'd like @wz1000 to check the tests make sense just to be sure!

michaelpj commented 1 month ago

nag @wz1000

soulomoon commented 3 weeks ago

Let's merge it if no one object to this. I've been using this branch and no regression yet. And the bench reuslt look pretty promising