Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sen Hastings <sen@phobosdpl.com>
To: buildroot@buildroot.org
Cc: sen@phobosdpl.com, thomas.petazzoni@bootlin.com
Subject: Re: [Buildroot] [PATCH v2 1/2] support/scripts/pkg-stats: move to procedurally generated row/column clases
Date: Tue, 30 Aug 2022 01:32:32 -0500	[thread overview]
Message-ID: <20220830063232.1671322-1-sen@phobosdpl.com> (raw)
In-Reply-To: <20220806235127.220d5546@windsurf>

On 8/6/22 16:51, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri,  5 Aug 2022 15:01:13 -0500
> Sen Hastings <sen@phobosdpl.com> wrote:
> 
>> Rather than having pre-baked classes in the markup for sorting purposes,
>> all the cells in package-grid are iterated over at load time and given
>> a row/column class.
>>
>> example: https://sen-h.github.io/pkg-stats/b6f4cbddb14233c3ab3fdfea1e486c14871cfb2a.html
>>
>> Signed-off-by: Sen Hastings <sen@phobosdpl.com>
> 
> I am probably not versed enough into HTML/CSS/JS sorcery, but I don't
> understand the benefit of this. Could you explain a bit more?
> 


Sure.

It turns out it's a lot cheaper (smaller) to have the client just
iterate over every single cell and give it a row/column class selector
instead of shipping it out with them.

sortGrid() sorts by virtue of all the elements in a column sharing the
same class (which originally was also the column label)
and all the elements in a row also sharing the same class
(which originally was also the package name).

The problem (previously) was that some package names were duplicated across rows.

For instance look at these two cells in the package column:

<div id="package_barebox" class="package data barebox">boot/barebox/barebox.mk</div>

<div id="package_barebox" class="package data barebox">boot/barebox/barebox/barebox.mk</div>

these are the beginning cells of two rows and they both share the
*same* class selector "barebox".

Can't have that because we need every row to have a unique class
assigned to it.

Otherwise when we sort this happens:

|label         |     label    |     label    |...|     label    |
|class1        |    class1    |    class1    |...|class1        |
|class1        |    class1    |    class1    |...|class1        |

becomes:

|label         |     label    |     label    |...|     label    |
|class1        |    class1    |    class1    |...|class1        |class1        |    class1    |    class1    |...|class1        |

sortGrid pulls all elements by row class selector and gives them
an explicit grid-row assignment

So now we have 26 elements in a row instead of 13 :p

The fix at the time,

(see: https://git.buildroot.net/buildroot/commit/?id=786f8b45672ebdd432f6af5b7595d3d16013433b)

was to use the package path but with "_" instead of "/", so:
boot_barebox_barebox and boot_barebox_barebox_barebox
respectively.

Also class selectors can't start with numbers so we prepend an "_"
to for the sake of packages like 4th and 18xx-ti-utils.

The problem I noticed was that having

id="column_name__some_really_really_long_path_name" class="centered data _some_really_really_long_path_name"

for every element added up after 70k+ elements.

the old pkg-stats
(https://git.buildroot.net/buildroot/commit/?id=eae86599ca81c943821bac33f424669520a3fa8c)

when given the current package list gives me an html file sitting at
about 2.9MB.
example: https://sen-h.github.io/pkg-stats/eae86599ca81c943821bac33f424669520a3fa8c.html

The current pkg-stats gives me a whopping 7MB file.
example: https://sen-h.github.io/pkg-stats/c245575.html

but you can see this whittled down by [PATCH v2 1/2]:
https://sen-h.github.io/pkg-stats/b6f4cbddb14233c3ab3fdfea1e486c14871cfb2a.html
(3.2M)

and then by [PATCH v1 2/2]:
https://sen-h.github.io/pkg-stats/9ad05210dcd9e4fb6b6a45be87c0fbb3e022085b.html
(2.6M)

in essence, we go from:

	<div id="upstream_url__package_kodi-audiodecoder-vgmstream_kodi-audiodecoder-vgmstream" class="centered upstream_url data _package_kodi-audiodecoder-vgmstream_kodi-audiodecoder-vgmstream good_url"><a href="https://github.com/xbmc/audiodecoder.vgmstream">Link</a></div>

and then with [PATCH v2 1/2]:

        <div class="centered data good_url"><a href="https://github.com/xbmc/audiodecoder.vgmstream">Link</a></div>

which after our column/row class assignment script runs becomes:

        <div class="centered data good_url c10 ir755"><a href="https://github.com/xbmc/audiodecoder.vgmstream">Link</a></div>

and finally with [PATCH v2 2/2]:

        <div class="good_url"><a href="https://github.com/xbmc/audiodecoder.vgmstream">Link</a></div>

and once again after the script runs:

        <div class="good_url c10 ir755"><a href="https://github.com/xbmc/audiodecoder.vgmstream">Link</a></div>

and here's the old sorttable version just for comparison:

	<td class="centered missing_url invalid_url"><a href="https://github.com/xbmc/audiodecoder.vgmstream">invalid (err)</a></td>

So why not just ship the class selectors "cX irX" for every element
instead of generating them client side?

I experimented with that but once again it's a size thing.

Every order of magnitude (10-100-1000) adds another digit (character)
to the ir (initial row) class selector.

Right now it's about 300k which doesn't seem like much but could
really add up over time.

The pages being generated right now would be considered *quite* big by most
standards, so I'm really just trying to whittle everything down to the
absolute bare minimum. 
 
>>  function sortGrid(sortLabel){
>>  	let i = 0;
>>  	let pkgSortArray = [], sortedPkgArray = [], pkgStringSortArray = [], pkgNumSortArray = [];
>> @@ -774,14 +785,11 @@ function sortGrid(sortLabel){
>>  	const styleSheet = styleElement.sheet;
>>  
>>          columnValues.shift();
>> -        columnValues.forEach((listing) => {
>> +	columnValues.forEach((listing) => {
> 
> This is a spurious change.
> 
>>          if (fieldText == "see all " + fieldTotal + triangleDown){
>> -		field.firstElementChild.innerText = "see less " + fieldTotal + triangleUp;
>> +                field.firstElementChild.innerText = "see less " + fieldTotal + triangleUp;
> 
> This is also a spurious change.
> 
Shoot, those are pretty spurious. I'll go ahead and fix that.

[note: I originally sent this out on the 2022-08-08, 
but not as reply to the previous message,
so it just showed up as a normal submission to the mailing list.
Re-submitting it for the sake of posterity.]


> Thanks!
> 
> Thomas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2022-08-30  6:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-05 20:01 [Buildroot] [PATCH v2 1/2] support/scripts/pkg-stats: move to procedurally generated row/column clases Sen Hastings
2022-08-05 20:01 ` [Buildroot] [PATCH v1 2/2] support/scripts/pkg-stats: optimize CSS selector usage Sen Hastings
2022-08-06 21:55   ` Thomas Petazzoni via buildroot
2022-08-06 21:51 ` [Buildroot] [PATCH v2 1/2] support/scripts/pkg-stats: move to procedurally generated row/column clases Thomas Petazzoni via buildroot
2022-08-30  6:32   ` Sen Hastings [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-08-08 22:23 Sen Hastings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220830063232.1671322-1-sen@phobosdpl.com \
    --to=sen@phobosdpl.com \
    --cc=buildroot@buildroot.org \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox