From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bernhard Bitsch <bbitsch@ipfire.org>
To: development@lists.ipfire.org
Subject:
 Re: [PATCH] speed.cgi: reduce system load by copying two general-functions.
Date: Fri, 22 Oct 2021 12:30:22 +0200
Message-ID: <c89186b4-5985-3b6a-681f-95f2206fe3db@ipfire.org>
In-Reply-To: <E6CD5D9C-64E4-42D5-962C-64817789CAA4@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============5455284979587539675=="
List-Id: <development.lists.ipfire.org>

--===============5455284979587539675==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi Michael,

you are right from a formal sight.
But testing the patch shows a reduction of system load in my system (=20
~50% ).
So the impact is not neglectable.
Because we should not copy functions, as stated, we must review these=20
Perl modules. This will be an ongoing process. I fear, we will find some=20
copys scattered in various files.
One thing I noticed while reading the various sources, is the relation=20
of general-functions.pl and network-functions.pl. general-functions.pl=20
includes the latter ( everytime ) and contains also network related=20
functions. Maybe this is a start point for brushing up.

Regards,
Bernhard


Am 22.10.2021 um 12:07 schrieb Michael Tremer:
> Hello Arne,
>=20
> I do not think I am happy with this patch because it duplicates code.
>=20
> Whenever we change or fix anything in any of those copied functions, we *wi=
ll* forget the copy in speed.cgi.
>=20
> What is the reason we cannot load general-funcions.pl in speed.cgi?
>=20
> Loading a slightly larger Perl file like general-functions.pl cannot impact=
 the performance of the interpreter. If it does, the way should be to fix any=
 slow parts in it.
>=20
> -Michael
>=20
>> On 22 Oct 2021, at 09:05, Arne Fitzenreiter <arne_f(a)ipfire.org> wrote:
>>
>> include general-functions.pl load and initialize many subfunctions that ar=
e not
>> needed by speed.cgi which was executed very ofthen.
>> So this reduce the system load significant if webif was open in browser
>> and ajax-speed display enabled.
>>
>> Signed-off-by: Arne Fitzenreiter <arne_f(a)ipfire.org>
>> ---
>> config/cfgroot/general-functions.pl |  2 ++
>> html/cgi-bin/speed.cgi              | 46 +++++++++++++++++++++++++++--
>> 2 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-=
functions.pl
>> index f72d6588c..c10a04faa 100644
>> --- a/config/cfgroot/general-functions.pl
>> +++ b/config/cfgroot/general-functions.pl
>> @@ -53,6 +53,7 @@ sub system_background($) {
>> }
>>
>> # Returns the output of a shell command
>> +# if you change this also check speed.cgi that include a local copy for s=
ystemload reasons
>> sub system_output($) {
>> 	my @command =3D @_;
>> 	my $pid;
>> @@ -1227,6 +1228,7 @@ sub firewall_reload() {
>> }
>>
>> # Function which will return the used interface for the red network zone (=
red0, ppp0, etc).
>> +# if you change this also check speed.cgi that include a local copy for s=
ystemload reasons
>> sub get_red_interface() {
>>
>> 	open(IFACE, "${General::swroot}/red/iface") or die "Could not open /var/i=
pfire/red/iface";
>> diff --git a/html/cgi-bin/speed.cgi b/html/cgi-bin/speed.cgi
>> index b550fda52..651c3c0b3 100644
>> --- a/html/cgi-bin/speed.cgi
>> +++ b/html/cgi-bin/speed.cgi
>> @@ -19,7 +19,47 @@
>> #                                                                         =
    #
>> ##########################################################################=
#####
>>
>> -require '/var/ipfire/general-functions.pl';
>> +#########################################################################=
######
>> +# functions copied from general-functions.pl for speed improvement becaus=
e    #
>> +# loading and initializing the whole general-functions.pl every second cr=
eate #
>> +# high system load                                                       =
     #
>> +###########################################OA############################=
######
>> +#
>> +# Returns the output of a shell command
>> +sub General__system_output($) {
>> +    my @command =3D @_;
>> +    my $pid;
>> +    my @output =3D ();
>> +
>> +    unless ($pid =3D open(OUTPUT, "-|")) {
>> +            open(STDERR, ">&STDOUT");
>> +            exec { ${command[0]} } @command;
>> +            die "Could not execute @command: $!";
>> +    }
>> +
>> +    waitpid($pid, 0);
>> +
>> +    while (<OUTPUT>) {
>> +            push(@output, $_);
>> +    }
>> +
>> +    close(OUTPUT);
>> +    return @output;
>> +}
>> +#
>> +# Function which will return the used interface for the red network zone =
(red0, ppp0, etc).
>> +sub General__get_red_interface() {
>> +
>> +     open(IFACE, "/var/ipfire/red/iface") or die "Could not open /var/ipf=
ire/red/iface";
>> +
>> +     my $interface =3D <IFACE>;
>> +     close(IFACE);
>> +     chomp $interface;
>> +
>> +     return $interface;
>> +}
>> +#
>> +#########################################################################=
######
>>
>> my $data_last =3D $ENV{'QUERY_STRING'};
>> my $rxb_last =3D 0;
>> @@ -38,8 +78,8 @@ foreach $field (@fields) {
>> 	}
>> }
>>
>> -my $interface =3D &General::get_red_interface();
>> -my @data_now =3D &General::system_output("ip", "-s", "link", "show", "$in=
terface");
>> +my $interface =3D &General__get_red_interface();
>> +my @data_now =3D &General__system_output("ip", "-s", "link", "show", "$in=
terface");
>>
>> my $lastline;
>> my $rxb_now =3D 0;
>> --=20
>> 2.25.1
>>
>=20

--===============5455284979587539675==--