From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@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 11:07:10 +0100
Message-ID: <E6CD5D9C-64E4-42D5-962C-64817789CAA4@ipfire.org>
In-Reply-To: <20211022080532.3195-1-arne_f@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============5799760955748406247=="
List-Id: <development.lists.ipfire.org>

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

Hello Arne,

I do not think I am happy with this patch because it duplicates code.

Whenever we change or fix anything in any of those copied functions, we *will=
* forget the copy in speed.cgi.

What is the reason we cannot load general-funcions.pl in speed.cgi?

Loading a slightly larger Perl file like general-functions.pl cannot impact t=
he performance of the interpreter. If it does, the way should be to fix any s=
low parts in it.

-Michael

> On 22 Oct 2021, at 09:05, Arne Fitzenreiter <arne_f(a)ipfire.org> wrote:
>=20
> include general-functions.pl load and initialize many subfunctions that are=
 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.
>=20
> 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(-)
>=20
> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-f=
unctions.pl
> index f72d6588c..c10a04faa 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -53,6 +53,7 @@ sub system_background($) {
> }
>=20
> # Returns the output of a shell command
> +# if you change this also check speed.cgi that include a local copy for sy=
stemload reasons
> sub system_output($) {
> 	my @command =3D @_;
> 	my $pid;
> @@ -1227,6 +1228,7 @@ sub firewall_reload() {
> }
>=20
> # Function which will return the used interface for the red network zone (r=
ed0, ppp0, etc).
> +# if you change this also check speed.cgi that include a local copy for sy=
stemload reasons
> sub get_red_interface() {
>=20
> 	open(IFACE, "${General::swroot}/red/iface") or die "Could not open /var/ip=
fire/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 @@
> #                                                                          =
   #
> ###########################################################################=
####
>=20
> -require '/var/ipfire/general-functions.pl';
> +##########################################################################=
#####
> +# functions copied from general-functions.pl for speed improvement because=
    #
> +# loading and initializing the whole general-functions.pl every second cre=
ate #
> +# 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/ipfi=
re/red/iface";
> +
> +     my $interface =3D <IFACE>;
> +     close(IFACE);
> +     chomp $interface;
> +
> +     return $interface;
> +}
> +#
> +##########################################################################=
#####
>=20
> my $data_last =3D $ENV{'QUERY_STRING'};
> my $rxb_last =3D 0;
> @@ -38,8 +78,8 @@ foreach $field (@fields) {
> 	}
> }
>=20
> -my $interface =3D &General::get_red_interface();
> -my @data_now =3D &General::system_output("ip", "-s", "link", "show", "$int=
erface");
> +my $interface =3D &General__get_red_interface();
> +my @data_now =3D &General__system_output("ip", "-s", "link", "show", "$int=
erface");
>=20
> my $lastline;
> my $rxb_now =3D 0;
> --=20
> 2.25.1
>=20


--===============5799760955748406247==--