8 ms·

Improvements merged within the last two days by the WebKit team: https://github.com/orgs/community/discussions/170922#discussioncomment-14229541 https://github.

by muglug 1y ago

Improvements merged within the last two days by the WebKit team: https://github.com/orgs/community/discussions/170922#discussioncomment-14229541 https://github.com/orgs/community/discussions/170922#discuss... For my sins I occasionally create large PRs (> 1,000 files) in GitHub, and teammates (who mostly all use Chrome) will sometimes say "I'll approve once it loads for me..."

patrickmay 1y ago
That seems essentially unreviewable. If you can share without violating an NDA, what kind of PR would involve that many files?
- trenchpilgrim 1y ago
  Ones where you have a lot of generated files you commit into Git, and you change the output of the generator tool.
- codezero 1y ago
  Can’t speak for the person above but we keep a lot of configuration files in git and could easily write a thousand new configs in a single PR, or adding a new key to all the configs for example.
- scsh 1y ago
  If the project you're working on vendors dependencies it's pretty easy to end up with that many files being changed when adding or updating, even when trying to make as narrow updates as possible in one PR.
- bob1029 1y ago
  "Upgrade solution from .NET Framework 4.8 => .NET 8" "Rename 'CustomerEmailAddress' to 'CustomerEmail'" "Upgrade 3rd party API from v3 to v4" I genuinely don't get this notion of a "max # of files in a PR". It all comes off to me as post hoc justification of really shitty technology decisions at GitHub.
  - ambicapter 1y ago
    It's not GitHub-specific advice, it's about reviewability of the PR vs. human working memory/maximum attention span.
    - eviks 1y ago
      How much working memory/attention span is required to look through 1000 identical lines "-CustomerEmailAddress +CustomerEmail"?
      - shadowgovt 1y ago
        Ideally, you automate a check like that. Because the answer turns out to actually be "humans are profoundly bad at that kind of pattern recognition." A computer will be able to tell that the 497th has a misspelled `CusomerEmail` or that change 829 is a regexp failure that trimmed the boolean "CustomerEmailAddressed" to "CustomerEmailed" with 100% reliability; humans, not so much.
        makeitdouble 1y ago
        You're not just reviewing the individual lines, but also which context, and which files are impacted. And automating that part would still mean reviewing the automation and the 1000+ changes to move to it. Sure 1000+ changes kills the soul, we're not good at that, but sometimes there's just no other decent choice.
        eviks 1y ago
        Oh, certainly, didn't mean that you had to avoid using your IDE to autorename a variable yourself (to avoid the boolean issue) and diffed results to those of the PR Or that you had to avoid Ctrl+F "CustomerEmail" and see whether you had 1000 matches that matches the number of changed files or only 999 due to some typo. Or using the web interface to filter by file type to batch your reviews. Or... Just that in none of those cases there is anything close to our memory/attention capacity.
- cesarb 1y ago
  > what kind of PR would involve that many files? A very simple example: migrating from JavaEE to JakartaEE. Every single Java source file has to have the imports changed from "javax." to "jakarta.", which can easily be thousands of files. It's also easy to review (and any file which missed that change will fail when compiling on the CI).
- moffkalast 1y ago
  Convert space indents to tabs, as god intended.
  - itsgabriel 1y ago
    as god indented, you mean?
    - moffkalast 1y ago
      I'm stealing that one, lmao
celsoazevedo 1y ago
How long until those improvements reach users? I assume it requires an OS update or does Safari use something similar to Firefox and Chrome for faster updates?
- rootnod3 1y ago
  There is a developer version you can install. There is beta, but that overrides your existing Safari and rollback might be tricky sometimes. But there is also the Safari Technology Preview, which installs as a separate app, but is also a bit more unstable. Similar to Chrome Canary.
  - philistine 1y ago
    STP is a great thing if you wished you had two different Safaris. Profiles just don't work as well as a completely different app.
  - dylan604 1y ago
    I had to download STP for a specific case I don't even remember. Ever since, I get frequent OS Update notifications with new STP versions. Updates without a fully system which means no rebooting necessary. About as easy any other software typically does it, only this is using the OS' upgrade so it does make it those extra steps instead of clicking the update->relaunch button
blinkingled 1y ago
Thanks, that is definitely a good sign - given the rendering engine monopoly state of Chrome+derivatives and lack of great momentum behind Firefox adoption we need Apple to actively keep Safari not just viable but great even if only on macOS/iOS.
Sesse__ 1y ago
Interesting how _everyone_ here blames JS and React, yet the fixes you linked are about CSS performance.
- jchw 1y ago
  You certainly can build slow apps with React, it doesn't make building slow things that hard. But honestly, React primitives (component mounting/unmounting, rendering, virtual DOM diffing, etc.) just aren't that slow/inefficient and using React in a fairly naive way isn't half-bad for data-heavy apps. I actually have been trying to figure out how to get my React application (unreleased) to perform less laggy in Safari than it does in Firefox/Chrome, and it seems like it is related to all the damn DOM elements. This sucks. Virtualizing viewports adds loads of complexity and breaks some built-in browser features, so I generally prefer not to do it. But, at least in my case, Safari seems to struggle with doing certain layout operations with a shit load of elements more than Chrome and Firefox do.
  - Sesse__ 1y ago
    > You certainly can build slow apps with React, it doesn't make building slow things that hard. By all means. It sometimes feels like React is more the symptom than the actual issue, though. Personally I generally just like having less code; generally makes for fewer footguns. But that's an incredibly hard sell in general (and of course not the entire story).
- ajross 1y ago
  Confirmation of priors is a powerful drug. And performance engineering is really hard and often lives at a different layer of the stack than the one you know. It's just easier to blame the tools (or companies!) you already hate.
- psygn89 1y ago
  JS is the logical place to start with all the virtualization and fanciness. But CSS has bit me with heavy pages (causing a few seconds of lag that even devtools debugging/logging didn't point towards). We know wildcard selectors can impact performance, but in my case there were many open ended selectors like `:not(.what) .ever` where the `:not()` not being attached to anything made it act like a wildcard with conditions. Using `:has()` will do the same with additional overhead. Safari was the worst at handling large pages and these types of selectors and I noticed more sluggishness 2-3 years ago.
deleted 1y ago
[deleted]
darkwater 1y ago
So, it was actually a bug in Safari's WebKit exposed by Github (which might be abusing it a bit anyway). But hey, since we are all web developers or web users, it's easier to shoot on the pianist (Github in this case)

by muglug 1y ago

patrickmay 1y ago

That seems essentially unreviewable. If you can share without violating an NDA, what kind of PR would involve that many files?

trenchpilgrim 1y ago
Ones where you have a lot of generated files you commit into Git, and you change the output of the generator tool.
codezero 1y ago
Can’t speak for the person above but we keep a lot of configuration files in git and could easily write a thousand new configs in a single PR, or adding a new key to all the configs for example.
scsh 1y ago
If the project you're working on vendors dependencies it's pretty easy to end up with that many files being changed when adding or updating, even when trying to make as narrow updates as possible in one PR.
bob1029 1y ago
"Upgrade solution from .NET Framework 4.8 => .NET 8" "Rename 'CustomerEmailAddress' to 'CustomerEmail'" "Upgrade 3rd party API from v3 to v4" I genuinely don't get this notion of a "max # of files in a PR". It all comes off to me as post hoc justification of really shitty technology decisions at GitHub.
- ambicapter 1y ago
  It's not GitHub-specific advice, it's about reviewability of the PR vs. human working memory/maximum attention span.
  - eviks 1y ago
    How much working memory/attention span is required to look through 1000 identical lines "-CustomerEmailAddress +CustomerEmail"?
    - shadowgovt 1y ago
      Ideally, you automate a check like that. Because the answer turns out to actually be "humans are profoundly bad at that kind of pattern recognition." A computer will be able to tell that the 497th has a misspelled `CusomerEmail` or that change 829 is a regexp failure that trimmed the boolean "CustomerEmailAddressed" to "CustomerEmailed" with 100% reliability; humans, not so much.
      - makeitdouble 1y ago
        You're not just reviewing the individual lines, but also which context, and which files are impacted. And automating that part would still mean reviewing the automation and the 1000+ changes to move to it. Sure 1000+ changes kills the soul, we're not good at that, but sometimes there's just no other decent choice.
      - eviks 1y ago
        Oh, certainly, didn't mean that you had to avoid using your IDE to autorename a variable yourself (to avoid the boolean issue) and diffed results to those of the PR Or that you had to avoid Ctrl+F "CustomerEmail" and see whether you had 1000 matches that matches the number of changed files or only 999 due to some typo. Or using the web interface to filter by file type to batch your reviews. Or... Just that in none of those cases there is anything close to our memory/attention capacity.
cesarb 1y ago
> what kind of PR would involve that many files? A very simple example: migrating from JavaEE to JakartaEE. Every single Java source file has to have the imports changed from "javax." to "jakarta.", which can easily be thousands of files. It's also easy to review (and any file which missed that change will fail when compiling on the CI).
moffkalast 1y ago
Convert space indents to tabs, as god intended.
- itsgabriel 1y ago
  as god indented, you mean?
  - moffkalast 1y ago
    I'm stealing that one, lmao

celsoazevedo 1y ago

How long until those improvements reach users? I assume it requires an OS update or does Safari use something similar to Firefox and Chrome for faster updates?

rootnod3 1y ago
There is a developer version you can install. There is beta, but that overrides your existing Safari and rollback might be tricky sometimes. But there is also the Safari Technology Preview, which installs as a separate app, but is also a bit more unstable. Similar to Chrome Canary.
- philistine 1y ago
  STP is a great thing if you wished you had two different Safaris. Profiles just don't work as well as a completely different app.
- dylan604 1y ago
  I had to download STP for a specific case I don't even remember. Ever since, I get frequent OS Update notifications with new STP versions. Updates without a fully system which means no rebooting necessary. About as easy any other software typically does it, only this is using the OS' upgrade so it does make it those extra steps instead of clicking the update->relaunch button

blinkingled 1y ago

Thanks, that is definitely a good sign - given the rendering engine monopoly state of Chrome+derivatives and lack of great momentum behind Firefox adoption we need Apple to actively keep Safari not just viable but great even if only on macOS/iOS.

Sesse__ 1y ago

Interesting how _everyone_ here blames JS and React, yet the fixes you linked are about CSS performance.

jchw 1y ago
You certainly can build slow apps with React, it doesn't make building slow things that hard. But honestly, React primitives (component mounting/unmounting, rendering, virtual DOM diffing, etc.) just aren't that slow/inefficient and using React in a fairly naive way isn't half-bad for data-heavy apps. I actually have been trying to figure out how to get my React application (unreleased) to perform less laggy in Safari than it does in Firefox/Chrome, and it seems like it is related to all the damn DOM elements. This sucks. Virtualizing viewports adds loads of complexity and breaks some built-in browser features, so I generally prefer not to do it. But, at least in my case, Safari seems to struggle with doing certain layout operations with a shit load of elements more than Chrome and Firefox do.
- Sesse__ 1y ago
  > You certainly can build slow apps with React, it doesn't make building slow things that hard. By all means. It sometimes feels like React is more the symptom than the actual issue, though. Personally I generally just like having less code; generally makes for fewer footguns. But that's an incredibly hard sell in general (and of course not the entire story).
ajross 1y ago
Confirmation of priors is a powerful drug. And performance engineering is really hard and often lives at a different layer of the stack than the one you know. It's just easier to blame the tools (or companies!) you already hate.
psygn89 1y ago
JS is the logical place to start with all the virtualization and fanciness. But CSS has bit me with heavy pages (causing a few seconds of lag that even devtools debugging/logging didn't point towards). We know wildcard selectors can impact performance, but in my case there were many open ended selectors like `:not(.what) .ever` where the `:not()` not being attached to anything made it act like a wildcard with conditions. Using `:has()` will do the same with additional overhead. Safari was the worst at handling large pages and these types of selectors and I noticed more sluggishness 2-3 years ago.

deleted 1y ago

[deleted]

darkwater 1y ago

So, it was actually a bug in Safari's WebKit exposed by Github (which might be abusing it a bit anyway). But hey, since we are all web developers or web users, it's easier to shoot on the pianist (Github in this case)