shorebirdtech / shorebird

Code Push for Flutter and other tools for Flutter businesses.
https://shorebird.dev
Other
1.97k stars 118 forks source link

Adding/Deleting Classes Negatively Impacts Link Percentage #1825

Closed felangel closed 2 weeks ago

felangel commented 1 month ago

As a developer, when I add or delete a class and publish the changes as a patch, I expect the application to still retain a relatively high link percentage (and consequently near native performance). Currently, we are aware of linking issues when classes are modified (link percentages drop down to (~10-20%) even for simple changes which touch classes.

eseidel commented 1 month ago

The theory here is that Dart presumably has a global dispatch table (I've not found it yet, but haven't looked very hard) and adding/removing a class adds or removes an entry from the class id list (similar to what constants did for the Object Pool) thus invalidating all ids after the added or removed class (now shifted down/up one value), thus invalidating (causing to not link) any code which references those classes. Which is how we suspect link percentages are dropping by 90% for a single line change like this. That's all theory, need to stare at our linker debugger a bit and go find the code in Dart which is responsible for this.

eseidel commented 3 weeks ago

Felix has looked into this, and found it's actually two separable issues. The ClassTable and DispatchTable both renumber all their rows, causing any classes/selectors after the inserted ones to get new numbers, and thus all code that interacts with them to change and become unlinkable (with our current linker). He's working on a fix to sort the ClassTable, which should be done this week (or early next) and then we'll look at the DispatchTable in greater detail. The ClassTable fix we expect to get about 25% on our linking benchmarks and should see a similar (large) win in the wild. The ClassTable fix is basically fixing all functions which reference a class in some form (e.g. allocating one) but don't call any methods on it to be able to link.

eseidel commented 3 weeks ago

I've been tracking updates in https://github.com/shorebirdtech/shorebird/issues/1892