Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Apple Silicon #771

Closed
wants to merge 3 commits into from
Closed

Conversation

ayroblu
Copy link
Contributor

@ayroblu ayroblu commented Jan 10, 2021

Inside Podfile, update Sparkle and AppCenter, then pod install

Also, AppCenter requires a major version update, and so requires following this migration guide

I set the release to use a self signed certificate (not included in this PR) and build release works

Copy link
Owner

@lwouis lwouis left a comment

Choose a reason for hiding this comment

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

I see that you updated some libraries, but how does that connect to the title of the PR which is about Apple Silicon? I'm sorry if I missed it! 😅

@@ -3,8 +3,8 @@ platform :osx, '10.12'
target 'alt-tab-macos' do
use_frameworks!
pod 'LetsMove', :git => 'https://github.com/lwouis/LetsMove.git', :commit => '7abf4daed1a25218f2b52f2dfd190aee5a50071c'
pod 'Sparkle', :podspec => 'https://raw.githubusercontent.com/lwouis/Sparkle/master/Sparkle.podspec'
pod 'Sparkle', '1.24.0'
Copy link
Owner

Choose a reason for hiding this comment

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

The reason I was pointing to a fork of Sparkle, is because in that fork, I addressed an issue that Sparkle has, that impacts release notes in AltTab. See the 3 commits I added in that fork. Moving back to upstream may create a regression here

Copy link
Owner

@lwouis lwouis Jan 15, 2021

Choose a reason for hiding this comment

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

Actually I checked Sparkle 1.24.0 release notes, and they have merged my fix (I had shared a PR with them back then). The issue is that since then I've updated my fix further. I should share my improvements in a PR, and re-fork Sparkle in the meanwhile.

@ayroblu
Copy link
Contributor Author

ayroblu commented Jan 11, 2021

XCode by default builds a release version which is "universal" on Big Sur. This means there's very little to do for Apple Silicon support.

And yes sorry I wasn't clear, this PR is just to illustrate what's needed for support as upstream libraries needed updates. Obviously you've made changes to sparkle, I can't comment on whether this will break for Sparkle, but you could rebase your branch etc

@lwouis
Copy link
Owner

lwouis commented Jan 15, 2021

XCode by default builds a release version which is "universal" on Big Sur.

Is it needed to run BigSur? The CI/CD which releases AltTab runs on an older macOS version. I don't release from my local machine. Changing these things represent considerable work/risk

@ayroblu
Copy link
Contributor Author

ayroblu commented Jan 15, 2021

Yeah arm support was added with Big Sur, so if you're running old macOS then you're running old XCode and it won't be able to create an arm binary.

I can't comment on risks, I have no clue how you'd test the package upgrades, you'll be better placed for that

@lwouis
Copy link
Owner

lwouis commented Jan 15, 2021

Yeah arm support was added with Big Sur, so if you're running old macOS then you're running old XCode and it won't be able to create an arm binary.

It seems that I need XCode 12 to build ARM. XCode is available on 10.15+ it seems. I'll try to download it and build a Universal App.

@lwouis
Copy link
Owner

lwouis commented Jan 15, 2021

Another issue is that 3 pods are not updated in this PR: LetsMove, SwiftyMarkdown, ShortcutRecorder. I think without every single pod binary universal, the app would run in Rosetta on ARM. So that's an issue.

@lwouis lwouis mentioned this pull request Jan 15, 2021
@ayroblu
Copy link
Contributor Author

ayroblu commented Jan 15, 2021

I mean I would expect it to throw a cold hard error in terms of support, generally speaking, if it compiles it should be fine unless it relies on some inherently intel things (fan monitoring etc). Given that people have said this works under rosetta, I would assume that if it compiles, it probably works, especially for something like a markdown renderer

@mnin
Copy link
Contributor

mnin commented Feb 26, 2021

@lwouis is there anything that can be done here to push this pull request forward?

@lwouis
Copy link
Owner

lwouis commented Feb 26, 2021

@mnin Apple Silicon is a new thing. This means the userbase is small, and the platform still young and unfamiliar. I'm taking this adoption slowly. Supporting Apple Silicon means:

  • Universal build vs 2 builds
    • Universal build means we now increase the size of the app for everyone
    • 2 builds means twice the CI/CD, releases to handle, channels to release to and auto-update from, explanations on the website, QA to test each release, issues unique to each platforms, etc
  • QA
    • I don't own a AS mac. This means I can't QA the releases. I'm currently the only one paying attention to quality control on this project. So if I'm not there to test things, we may have big regressions on AS in any release, including the first one
    • I would have to test AS + Intel on every following release

These are a few reasons why adopting a whole new platform is a big deal, and I'm not rushing it.

If you want to fast-track this, then these issues should be addressed. That means people contributing some AS hardware to me or contributing their time to QA new releases, maybe a PR to help setup the new CI/CD pipelines, etc.

@ayroblu
Copy link
Contributor Author

ayroblu commented Feb 26, 2021

Just for my own curiousity, do you want to create a release build off a (rebased?) version of this PR and see if it works well for anyone who wants to try? My belief is that it should just work, but you never know

@lwouis
Copy link
Owner

lwouis commented Mar 1, 2021

@ayroblu yes feel free to make a local build and share it here. I'm not sure how many people here would be interested. For now we are 3 people talking in this PR. But yeah it's a good idea to provide a local build here that people with an AS mac can play with 👍

@mnin
Copy link
Contributor

mnin commented Mar 1, 2021

Here is my running version for testing purposes.

This includes both architectures for x86_64 and arm64.

I left two things here on purpose:

  • The version number is not set "#VERSION#".
  • The menu option to start AltTab automatically is removed

AltTab.zip

@inprealpha
Copy link

inprealpha commented Mar 2, 2021

Can confirm that @mnin's build is working on my M1 Air natively.

If there's any specific functionality I should test, let me know.

@lwouis
Copy link
Owner

lwouis commented Mar 2, 2021

@inprealpha cool! How does it compare to the rosetta-emulated version? I'm imagining that it might actually be indistinguishable since the app is light on CPU and is mostly doing system API calls.

It would be nice to compare latency and other performance aspects of emulated vs native.

@mnin
Copy link
Contributor

mnin commented Mar 2, 2021

@lwouis I have the feeling that it runs snappier. The CPU usage seems to be a bit lower.

How would you want to test the latency?

Do you have any drawbacks with this version?

@lwouis
Copy link
Owner

lwouis commented Mar 3, 2021

@mnin yes supporting a new platform represent a lot of additional work as a one-off but also ongoing basis. See my comment above for some examples of implications.

To measure latency, what I do is point a high-speed camera at my screen. For instance my iphone does slow-mo recording. I then review the footage frame-by-frame, and count how many camera frames it took from keyboard actuation to the UI showing on screen.

Here it would be nice to compare a native vs an emulated build, and see if there is any difference in latency.

The CPU usage seems to be a bit lower

CPU usage is incredibly hard to make sense of to be honest. It depends on your actual hardware, and the workflow (AltTab has more work if you have 2 4K monitors with 50 windows, compared to 1 laptop screen with 2 windows).

It would also be interesting to compare some kind of average load generated by both versions. Now how to measure that average load is an open topic. I usually use iStats Pro and open it and see AltTab going up to maybe 8% when I summon it, but that's on my machine, and with my monitor. And how long does it stay at 8%, etc. I did a lot of more precise/scientific performance work in Instruments, but here we are talking about performance of the end-user .app, so instrumentation is not possible, thus the high-speed camera, iStats Pro approach.

@inprealpha
Copy link

@lwouis Did the latency test twice on my Galaxy 960fps. It came roughly 17 and 20 frames after pressing tab on my keyboard. Hope that's helpful.

Regarding the CPU usage, I tried it using both Rosetta (x86_64) and native. I conducted 20 trials on each, using activity monitor (update freq set to very often). Using Rosetta the average %CPU (as I understand that's the %CPU used in a single core) was around 4.4%. When running native however, this dropped to 2.7%.

@lwouis
Copy link
Owner

lwouis commented Mar 3, 2021

@inprealpha thanks for the study!

I think the first conclusion to draw is that benefits of a native build are very small. It's like a CPU flag to get 10% optimization. Nice to have, but not a game changer, and i would assess not worth the efforts and downsides described above like increasing the app size

@inprealpha
Copy link

@lwouis Understandable.

It'd cool if someone maintained unofficial builds or something though. Obviously I don't take that for granted!

@mnin
Copy link
Contributor

mnin commented Mar 4, 2021

@lwouis @inprealpha @ayroblu

I'm not sure if file size is really an issue. The app is about 23 megabytes, as a ZIP archive 8.2 megabytes. And I clearly feel that the arm64 version is more responsive on my M1 Mac.

Attached is a new build with the latest changes from the master branch.

AltTab.zip

@lwouis
Copy link
Owner

lwouis commented Mar 5, 2021

@mnin could you try and put some numbers on that feeling of snappiness so we understand the benefits. Results from @inprealpha were pretty unimpressive, so i wonder if it's different on your machine

@ayroblu
Copy link
Contributor Author

ayroblu commented Mar 5, 2021

Not sure why I was tagged. I generally agree, file size doesn't matter much, Universal Binary works well as it's mostly a compiler issue so compile time checks appear to be sufficient. I'd probably add an understanding of "unofficial support" for Apple Silicon in the same way we might support different versions of macOS or websites have unofficial support for different browsers which "should work in theory" but don't have a CI setup. Also yes setting up CI is tricky, though based on GitHub actions I've seen I probably could setup an auto forked release. I have to maintain a fork anyways for an Alfred integration I want which relies on setting up a HTTP server.

Anyways, I'm not the maintainer so frankly it's whatever the maintainer is comfortable with

@inprealpha
Copy link

@mnin Yeah imo the file size difference really doesn't feel like a big deal at all. Obviously if you apply that mindset to everything it will eventually lead to a really bloated file size, but Apple Silicon is clearly here to stay so I think it's more than fair tradeoff.

The experience with Rosetta isn't totally seamless yeah. When opening the app for the first time they give a "To open AltTab, you need to install Rosetta" prompt which takes a while then the first time you open the app it takes a while too. Not a super big deal though.

Snappiness hasn't been an issue for me. Although I tried the same cpu load test with a lot of windows open and then the native version was using 13% while the Rosetta one was using 27% CPU, so almost the double CPU usage. If I were to use the laptop on battery power, I would definitely prefer the native version.

@lwouis lwouis mentioned this pull request Mar 21, 2021
@decodism
Copy link
Contributor

Even if the distribution of a universal version is not planned at the moment, it would be nice to update the dependencies to be able to compile natively.

@lwouis
Copy link
Owner

lwouis commented Apr 22, 2021

@decodism

Even if the distribution of a universal version is not planned at the moment

The universal app route is probably where we are going to go eventually, as I don't see maintaining 2 different release channels. Same reasoning as in #158.

For now, there is a retro-compat tool built-in on AS macs, and there are not many users, so they can just run the current app in emulation mode. In a few months to 1 year probably, I'll start releasing a universal bundle with an actual AS arch binary in it.

it would be nice to update the dependencies to be able to compile natively.

That's what this PR is doing

@mnin
Copy link
Contributor

mnin commented May 7, 2021

Just rebased all changes from master and create a new build for testing purposes.

AltTab.zip

@merken
Copy link

merken commented May 10, 2021

@mnin that latest version seems to be working on M1 MBA here. 👍

@paul-nameless
Copy link

paul-nameless commented Oct 25, 2021

🙏 Please make universal build

@barhom
Copy link

barhom commented Nov 1, 2021

I second this, please make the universal build ;)

@DaPutzy
Copy link

DaPutzy commented Nov 4, 2021

New build 6.26.0

Thanks mate. Works flawlessly. (MBP 2021 with M1 Pro)

@mnin
Copy link
Contributor

mnin commented Nov 7, 2021

Hello all, I will continue to work on the PR #1094 starting next week. I think we will get the PR than merged soon.

@mnin
Copy link
Contributor

mnin commented Nov 12, 2021

Hey,

I have been able to solve the most discussed problem in my MR #1094.

To do this, you need to know that the way AltTab is automatically started when you log in has changed.

Before you download this version: If you are running the previous version, please disable the start on login with this old version and then delete the old version. And install the new one.

This version contains also the last commits of 6.27.0.

And keep in mind, this version is just for testing purposes, it's not the official stable release build, it maybe contains issues.

AltTab.zip

@lwouis
Copy link
Owner

lwouis commented Nov 13, 2021

I've been investigating (1, 2, 3) on my own branch where I made minimal changes to support apple silicon.

@mnin @DaPutzy @barhom @paul-nameless @ayroblu @albertorestifo @jenslys @RobSjaras @AJAYK-01 @lbibass @ChristianVaughn @stevehoek @merken @decodism @inprealpha Could you please test this universal build and let me know if it works correctly on your M1 mac?

It works correctly for on my intel mac, on the host macOS 10.15 and in my macOS 12 VM.

@AJAYK-01
Copy link

I've been investigating (1, 2, 3) on my own branch where I made minimal changes to support apple silicon.

@mnin @DaPutzy @barhom @paul-nameless @ayroblu @albertorestifo @jenslys @RobSjaras @AJAYK-01 @lbibass @ChristianVaughn @stevehoek @merken @decodism @inprealpha Could you please test this universal build and let me know if it works correctly on your M1 mac?

It works correctly for on my intel mac, on the host macOS 10.15 and in my macOS 12 VM.

It initially kept asking about granting screen recording permission although I had already given it.
Then I removed and gave permission again.
Now the app doesn't open at all 😟

@lwouis
Copy link
Owner

lwouis commented Nov 13, 2021

@AJAYK-01 the permission issue is probably because the previous AppleSilicon build you used was the one from @mnin which is not signed/notarized with my developer certificate.

You can reset all permissions to AltTab by running this in Terminal.app:

tccutil reset All com.lwouis.alt-tab-macos

Regarding the app not opening, could you please open it in Terminal.app and share the logs you get? Maybe also the crash report in Console.app if there is one?

@mnin
Copy link
Contributor

mnin commented Nov 13, 2021

I've been investigating (1, 2, 3) on my own branch where I made minimal changes to support apple silicon.
@mnin @DaPutzy @barhom @paul-nameless @ayroblu @albertorestifo @jenslys @RobSjaras @AJAYK-01 @lbibass @ChristianVaughn @stevehoek @merken @decodism @inprealpha Could you please test this universal build and let me know if it works correctly on your M1 mac?
It works correctly for on my intel mac, on the host macOS 10.15 and in my macOS 12 VM.

It initially kept asking about granting screen recording permission although I had already given it. Then I removed and gave permission again. Now the app doesn't open at all 😟

Here are the crash log on M1 with Big Sur:
AltTab_2021-11-13-125956_c36cd8cb-2375-426a-a880-a503a6e47415.crash.zip

@mnin
Copy link
Contributor

mnin commented Nov 13, 2021

Here are the crash log on M1 with Big Sur: AltTab_2021-11-13-125956_c36cd8cb-2375-426a-a880-a503a6e47415.crash.zip

And yes, it occurs because of using LSSharedFileListInsertItemURL.

@lwouis
Copy link
Owner

lwouis commented Nov 13, 2021

@mnin the crash report doesn't give enough info about why it crashed. Usually when I debug the app and it crashes, the debugger will stop on the exact line that crashed, and I can see nicely the exception message, and the state of the various variables. Could you maybe do that and share what exactly goes wrong with the call to LSSharedFileListInsertItemURL?

I was wondering if it was kLSSharedFileListItemBeforeFirst that's being the problem, or maybe that since they changed the optionality of some params in the LSSharedFileListInsertItemURL call, we have to slightly change the input params?

@mnin
Copy link
Contributor

mnin commented Nov 14, 2021

Just updated the build, see #771 (comment) to 6.27.1:
AltTab.zip

@lwouis
Copy link
Owner

lwouis commented Nov 15, 2021

Before you download this version: If you are running the previous version, please disable the start on login with this old version and then delete the old version. And install the new one.

If we move to this API, it would be nice to automate the step above. It seems that removing items from the Login Items doesn't crash, so we could do a one-time clean up to remove AltTab from the Login Item. There is migration code for the preferences already to give an idea of how i've done it in the past.

I always try to be very thoughtful about upgrades not breaking people's setups when i introduce breaking changes

@lwouis
Copy link
Owner

lwouis commented Nov 16, 2021

@mnin @DaPutzy @barhom @paul-nameless @ayroblu @albertorestifo @jenslys @RobSjaras @AJAYK-01 @lbibass @ChristianVaughn @stevehoek @merken @decodism @inprealpha I made a build which worked properly on my Intel Catalina mac as well as a M1 Monterey VM. Could you please test it out?

Please also test the "Start at login" feature. Please remove the AltTab row in System Preferences > User & Groups > Login Items before you test it. The new implementation starts at login but doesn't show in the Login Items.

For the official release, I'll try to add some migration code which removes the Login Item for users who just updated. It's turned on by default, so most users will have an item to be removed.

@DaPutzy
Copy link

DaPutzy commented Nov 16, 2021

Works as expected :) Removed it from Login Items and Start on login still worked. Tested on MBP 16" M1 Pro Monterey.

@barhom
Copy link

barhom commented Nov 16, 2021

@lwouis I've tested your release now by first deleting the login item, then updating the app.
It run smoothly.

I then rebooted the laptop (M1 14" Monterey) - alt-tab auto started correctly.

(PS: Under about the version says it is runnig #VERSION#)

Update: It crashed once, I sent the feedback

@paul-nameless
Copy link

paul-nameless commented Nov 16, 2021

Installed, looks like it's working fine. I'll write if face any issues during next couple of days.
MacBook Air M1 (Bit Sur)

@merken
Copy link

merken commented Nov 19, 2021

Testing right now, works like a charm. But I'm not a power-user... Anything specific you want me to test ?

@lwouis
Copy link
Owner

lwouis commented Nov 19, 2021

@merken yes please test the start-at-login feature. More details here: #771 (comment)

maybe also looking at how much CPU/memory the app consumes over time would be interesting

@lwouis
Copy link
Owner

lwouis commented Nov 19, 2021

I have combined all changes from the other PRs here: #1239. I've credited both Ben and Martin in the git commits, and I've added few commits and changes.

Regarding the main topic, native Apple Silicon support, I'll see if the new Travis env works to build. It shouldn't be a problem. I tested the migration from previous version, and it works nicely. I tested the build, and other people tested the build successfully.

Let me know if you think I shouldn't release that build. Otherwise I'll do it when Travis passes.

@merken
Copy link

merken commented Nov 19, 2021

@merken yes please test the start-at-login feature. More details here: #771 (comment)

maybe also looking at how much CPU/memory the app consumes over time would be interesting

CPU usage is low, not worth mentioning
Memory is about 89megs

After reboot, the app starts up normally
Pretty happy with the results so far.

@lwouis
Copy link
Owner

lwouis commented Nov 19, 2021

AltTab v6.28.0 has just been released, and has native Apple Silicon support. Enjoy! 🥳

@lwouis lwouis closed this Nov 19, 2021
@ayroblu ayroblu deleted the universal branch June 19, 2022 09:35
Copy link

@lilsunny243 lilsunny243 left a comment

Choose a reason for hiding this comment

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

Comment Leave

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.