SMF icon indicating copy to clipboard operation
SMF copied to clipboard

Memory exhaustion running board permissions with lots of groups/permissions

Open jdarwood007 opened this issue 2 years ago • 14 comments

Description

[Fri Nov 05 20:17:00.814668 2021] [php7:error] [pid 30976] [client xxxx:0] PHP Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 20480 bytes) in /Sources/Reports.php on line 976, referer: xxxx/index.php?action=admin;area=reports

Steps to reproduce

In general

  • About 150 boards
  • About 150 groups
  • About 50 General Permissions (Some groups are using inheritance)
  • 30 Permission Profiles
  • Using Post moderation
  1. Go to Admin center > Reports
  2. Choose Board Permissions
  3. Run

Environment (complete as necessary)

  • Version/Git revision: b11f632dd
  • Database Type: Mysql
  • Database Version:
  • PHP Version:

Additional information/references

jdarwood007 avatar Nov 06 '21 00:11 jdarwood007

Sonne kind of test data would be nice, To had better understanding of the this.

albertlast avatar Nov 06 '21 09:11 albertlast

I can't give that data to the public. But the numbers of groups/etc are estimates but should provide a good idea.

jdarwood007 avatar Nov 06 '21 12:11 jdarwood007

Interesting that it's running out of memory there. That's in the middle of a function to handle date formats.

Oldiesmann avatar Nov 06 '21 17:11 Oldiesmann

well this say less than you believe, since this place is only the last place who called for more memory, so other places could consume the main memory.

But since we had no information about the data, we can't produce a profile where used memory is shown.

albertlast avatar Nov 06 '21 18:11 albertlast

You do realize you can auto generate lots of fake data to reproduce this setup? I can't provide the data in public. I've offered the SMF Devs a copy of the relevant data but it has to remain kept between them. Just how data sensitivity is sometimes.

Sometimes we can't get our hands on the data, but we can also through some of the functions calls into performance testing iterators and see how much memory/time is used to perform X amount of iterations. Which may also lead to being able to figure out how to optimize the code better.

jdarwood007 avatar Nov 07 '21 02:11 jdarwood007

Hm. The only really significant changes to this function since the 2.1 branch was first created were #318 and #1174, neither of which seem like they could cause any massive increase in memory usage compared to 2.0. #318 would increase it a little bit, but only a little bit. I suspect that a similarly large dataset would exhaust memory resources for SMF 2.0.x as well.

From what I can tell, the problem is simply that we are trying to hold all of that data in memory at once. Avoiding this will probably require rewriting the relevant code to use some combination of a generator and caching the compiled data as we build it.

Sesquipedalian avatar Nov 08 '21 02:11 Sesquipedalian

Wonder if we could do logic similar to what we do for messages. We set a context that we use to iterate over all the messages and send them directly to output.

jdarwood007 avatar Nov 09 '21 00:11 jdarwood007

Yeah, probably. The prepareDisplayContext() function and its kin are essentially just poor man's generators, after all.

Sesquipedalian avatar Nov 09 '21 01:11 Sesquipedalian

Well before anyone tries to fix it, should we fix it this late in RC? I'm not sure we should or not. Or even if it should make a patch version.

jdarwood007 avatar Nov 09 '21 03:11 jdarwood007

I'm waiting for the next version to pop up before I submit my changes.

live627 avatar Nov 10 '21 05:11 live627

@live627 You saying you have a change here? I haven't started to write anything because I was sure this would be a heavy change. The only minimal change I was thinking was using --/++$var instead of $i--/++ (slightly faster). Thought maybe of trying to use pass by references, but we are modifying a global it seems, so the memory usage doesn't seem it would benefit from that. Not that PHP works like C/C++ in the regards to pointers and references, but research indicates it may be more memory efficient (at least in older versions, not sure if still true).

jdarwood007 avatar Nov 11 '21 01:11 jdarwood007

Passing parameters by reference isn't really a memory-saving hack because of copy-on-write.

Sara Golemon has an interesting article at http://blog.golemon.com/2007/01/youre-being-lied-to.html

The moral of the story

Assigning values by references when you don't need to ... is NOT a case of you outsmarting the silly engine and gaining speed and performance. It's the opposite, it's you TRYING to outsmart the engine and failing, because the engine is already doing a better job than you think.

live627 avatar Nov 11 '21 05:11 live627

For those of you wondering, I can confirm that when we were running 2.0 on SM.org - We did run these reports without issues a couple of years back on multiple occasions. The group setup on SM.org should not have grown that significantly in between, no new fun groups created that I know of.

LexArma avatar Nov 13 '21 08:11 LexArma

Moved to 2.1.3 for now, but if @live627's fix can be ready in time for 2.1.2, I will be glad to include it there.

Sesquipedalian avatar Apr 04 '22 18:04 Sesquipedalian