5 ms·

Yeah. I think as a straightforward, easily correct transition from 2000 globals, a giant structure isn't an awful idea. It's not like the globals were organiz

• mihaic 3y ago
  Exactly, if you're now forced to put everything in one place you're forced to acknowledge and understand the complexity of your state, and might have incentives to simplify it.
  • Sesse__ 3y ago
    Here's MySQL's all-session-globals-in-one-place-class: https://github.com/mysql/mysql-server/blob/8.0/sql/sql_class.h#L927 https://github.com/mysql/mysql-server/blob/8.0/sql/sql_class... I believe I can safely say that nobody acknowledges and understands the complexity of all state within that class, and that whatever incentives there may be to simplify it are not enough for that to actually happen. (It ends on line 4692)
    • IshKebab 3y ago
      Right but that would still be true if they were globals instead. Putting all the globals in a class doesn't make any difference to how much state you have.
      • Sesse__ 3y ago
        > Putting all the globals in a class doesn't make any difference to how much state you have. I didn't make any claims about the _amount_ of state. My claim was that “you're forced to acknowledge and understand the complexity of your state” (i.e., moving it all together in one place helps understanding the state) is plain-out wrong.
        • IshKebab 3y ago
          It's not wrong. Obviously putting it all in one place makes you consider just how much of it you have, rather than having it hidden away all over your code.
• stingraycharles 3y ago
  Yes, it’s the most pragmatic and it’s only “awful” because it makes the actual problem visible. And would likely encourage slowly refactoring code to handle its state in a more sane way, until you’re only left with the really gnarly stuff, which shouldn’t be too much anymore and you can put them in individual thread local storages. It’s an easy transition path.
• cakoose 3y ago
  > I think as a straightforward, easily correct transition from 2000 globals, a giant structure isn't an awful idea. Agree. > It's not like the globals were organized before! Using a struct with 2000 fields loses some encapsulation. When a global is defined in a ".c" file (and not exported via a ".h" file), it can only be accessed in that one ".c" file, sort of like a "private" field in a class. Switching to a single struct would mean that all globals can be accessed by all code. There's probably a way to define things that allows you to regain some encapsulation, though. For example, some spin on the opaque type pattern: https://stackoverflow.com/a/29121847/163832 https://stackoverflow.com/a/29121847/163832
  • pasc1878 3y ago
    No that is what a static in a .c file is for. A plain global can be accessed from other compiled units - agreed with no .h entry it is my=uch more error prone e.g. you don't know the type but the variables name is exposed to other objects
    • remexre 3y ago
      Wouldn't those statics also be slated for removal with this change?
      • junon 3y ago
        At most they'd be determined to be read only constants that are inlined during constant folding. This includes most integral sized / typed scalar values that fit into registers for the most part, and nothing you've taken the address of either - those remain as static data.
        • cakoose 3y ago
          I think there might be a terminology mix-up here. In C, a global variable with the `static` keyword is is still mutable. So it typically can't be constant-folded/inlined. The `static` modifier in that context just means that the symbol is not exported, so other ".c" files can't access it.
• cogman10 3y ago
  I think my bigger fear is around security. A process per connection keeps things pretty secure for that connection regardless of what the global variables are doing (somewhat hard to mess that up with no concurrency going on in a process). Merge all that into one process with many threads and it becomes a nightmare problem to ensure some random addon didn't decide to change a global var mid processing (which causes wrong data to be read).
  • dfox 3y ago
    All postgres processes run under the same system user and all the access checking happens completely in userspace.
    • fdr 3y ago
      Access checking, yes, but the scope of memory corruption does increase unavoidably, given the main thing the pgsql-hackers investigating threads want: one virtual memory context when toggling between concurrent work. Of course, there's a huge amount of shared space already, so a willful corruption can already do virtually anything. But, more is more.
• magicalhippo 3y ago
  We did this with a project I worked on. I came on after the code was mature. While we didn't have 2000 globals, we did have a non-trivial amount, spread over about 300kLOC of C++. We started by just stuffing them into a "context" struct, and every function that accessed a global thus needed to take a context instance as a new parameter. This was tedious but easy. However the upside was that this highlighted poor architecture. Over time we refactored those bits and the main context struct shrunk significantly. The result was better and more modular code, and overall well worth the effort in our case, in my opinion.