chore: Strip debuginfo symbols for release

[comphead]

Which issue does this PR close?

• Closes Datafusion binary size has been getting bigger #13816.

Minimizing the DF binary size from 60M to 50M by stripping debuginfo.

Removing unwind panic saves 10 more MB

The stripped binary has the biggest methods for std panic

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[rkrishn7]

🙌🏾

One consideration re: panic=abort is that it will cause the program to crash upon some task panicking. Unclear on whether this is more/less desirable than bubbling up the error to the user.

For example, if some panic occurs during writing to an external table - do we want to abort? Or maybe it's preferable to surface the error (as I think would be the current behavior but not completely sure).

[rkrishn7]

nit: I believe this is superfluous - https://doc.rust-lang.org/cargo/reference/profiles.html#debug

[comphead]

Agree

[comphead]

DF does not catch panics, so the process will crash anyway no matter what the setting is.
Although there are some test cases which does catch_unwind but we do run tests in debug mode.
Another thing for abort it doesn't call object destructors if process crashed which for DF imho it is not an issue.

Potentially DF even can gain some performance which I will check separately and share the results here

[rkrishn7]

DF does not catch panics, so the process will crash anyway no matter what the setting is.

@comphead If I'm not mistaken, DF may catch panics in certain cases.

For example, if a serialization task panics during writing to some object store (see here).

I confirmed this by inserting a dummy panic in the serialization task.

[comphead]

Hey @rkrishn7 this is a good call yes, although DF does not catch panics, we cannot say the same for the dependencies. I'll experiment later with panic mode for DF crate level only, leaving other dependencies compile flags intact.

But for this PR it is safer to remove abort on panics

[comphead]

@alamb can I have a quick review for this?

[alamb]

Let's try it! Given panics are meant to be uncommon, it seems reasonable to not make datafusion-cli 2x larger to produce stack traces there

If people miss them, we can put it back

Thanks @comphead

[alamb]

I think it would be useful to add some comments here about why we chose to abort on panic (to decrease the binary size)

[comphead]

I rolled back the panic behavior. the PR is for the strip only which is safe.

Panic compilation behavior is also applied to dependencies which can be shoot in the foot, and this parameter is not overridable on crate level
https://doc.rust-lang.org/cargo/reference/profiles.html#overrides

Overrides cannot specify the panic, lto, or rpath settings.

Although it saves extra 15-20% I dont think we can use it in near future