misc: Revert GlobalConfig to use gflags again

[kgpai]

Summary:
X-link: facebookincubator/nimble#147

Reverts the use of GlobalConfig and switches back to using gflags. Unfortunately using globalConfig requires us to call translateFlagsToGlobalConfig immediately after ParseCommandLineFlags to initialize it right. Because other libraries also use velox , transitively there are many such places. Thus reverting this change till we figure out a better mechanism to remove gflags dependencies.

Differential Revision: D70463150

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	9619454
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/67c4ad4ead99300008399b82

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D70463150

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D70463150

[kgpai]

cc: @majetideepak

[kgpai]

Our internal linter doesnt have any issues with this change . Will investigate that separately.

[facebook-github-bot]

This pull request has been merged in bbaf9b8.

[majetideepak]

@kgpai I was not expecting this to be reverted on short notice during the weekend.
The global config change was an important step toward releasing the Velox library.
This was done to avoid facing similar issues due to global gflags symbols in the folly library.
CC: @pedroerp, @mbasmanova

[mbasmanova]

@majetideepak Deepak, the problems we discovered required quick mitigation. Hence, Krishna proceeded with the revert without much delay. We need to take a step back and think about why we cannot use GFlags in Velox. GFlags is a widely used library. Are you saying no other library out there uses it? Is it really not usable within a library?

[majetideepak]

@mbasmanova Yes, GFlags are not recommended to be used in libraries since the flags have to be declared in the global namespace and can lead to symbol conflicts. We see this issue with folly when building the Velox shared library #10732 (comment) and also when working with the dynamic library registration #11439.
.... Linux compilation by preventing errors related to 'is being linked both statically and dynamically into this executable,' particularly for folly_hazptr_use_executor.

They also carry the same static initialization order problem if they are being used to initialize other global static variables.

You will see them mostly being used in an application.

[majetideepak]

I see that both DuckDB and Arrow do not use GFlags in the core library and use config instead.

[assignUser]

Additionally my understanding is that using gflags forces anyone that wants to use Velox (or parts of velox that require the flags at least?) to build against gflags when building their application and add gflags::ParseCommandLineFlags(&argc, &argv, true); to their main() (see e.g. MergeBenchmark.cpp). Which given that the last gflags release was 6 years ago would annoy me if I wanted to use for example a more up to date alternative.

So I agree with @majetideepak that we should remove the usage of gflags in libvelox (no problem using it in tests etc. ofc).

[kgpai]

@majetideepak Firstly apologies for reverting the change - all blame for this should lie on my doorstep. The reason I had to work on removing it was because Velox is used directly or transitively in many thousands of packages internally and consequently again in many services at meta. Currently unfortunately without the translateFlags option it is not possible to change the behavior and considering the number of services that use Velox , this was a ticking time bomb and as oncall I felt it prudent to revert . Please consider this as a temporary setback - I will work with you in ensuring that the next solution doesnt have these problems and works for all of us.

[kgpai]

@assignUser Currently though there are many thousands of places that ParseCommandLineFlags is used internally and there is a large percentage of these that use Velox. Gflags is pretty endemic and updating all the places where its used and might be a pathway for Velox use puts a large load on the Velox team.