Bug 105102

Attempting some naive rebaseing right now. f93fa33 looks a bit complicated with all the extra operands...

His work is a great example of how NOT contribute to an Open Source project.  What a shame...

Some are those are good to grab. Some are good ideas, but should be implemented in a less intrusive manner. Others are just premature optimization without any measurable impact IMO.

Done-ish at https://github.com/AOSC-Dev/fontconfig/tree/master-reb. Bai can you tell me whether it builds?

(In reply to Mingye Wang (Arthur2e5) from comment #4)
> Done-ish at https://github.com/AOSC-Dev/fontconfig/tree/master-reb. Bai can
> you tell me whether it builds?

It does build but we (for transparency's sake I'm currently working with Wang on this) are running into some weird segfaults.

(In reply to Behdad Esfahbod from comment #3)
> Some are those are good to grab. Some are good ideas, but should be
> implemented in a less intrusive manner. Others are just premature
> optimization without any measurable impact IMO.

Right, in our interest we will continue to work on merging it with current master. However, if you would like to help/pick out changes you find useful, please do so by all means - after all it is not wise for us to keep a fork.

Here's my take on the optimizations. First a note: KDE's use of FcFontMatch() with FC_FILE set is bogus and broken. Ignore that.

* 3.4.1 Rewriting FcFontMatch algorithm
-> Valid idea. 10% is worth taking.

* 3.4.2 Value preprocessing
-> Valid concern. It's been on my radar for years. The solution is too invasive for my taste. Another approach might be to add new pattern elements FC_FAMILY_HASH, etc, that retain the (corresponding type of) hash of the string. It will be equally intrusive I'm afraid. But yeah, definitely something to pursue and see if better implementation is possible. Definitely biggest waste of time in current FontConfig.  Side note: if we go all the way to keep hashes of family names, we might end up wanting to use a hashtable for FcFontMatch...

* 3.4.3 Reducing heap allocations
-> Not clear that is measurable. But sure, is not intrusive. I'm open to.

* 3.4.4 Refactoring FcStrCaseWalkerNext
-> Sounds fine. We might not need to triplicate it, maybe just inline that function.

* 3.4.5 Micro-optimization in FcCompareValueList
-> I have a hard time believing the 38% speedup claimed. If it has measurable impact, sounds good to me.

* 3.4.6 Reduce FcObjectFromName call amount
-> Not sure it's worth it; has no measurable impact.

* 3.4.7 Hint branch predictor
-> Good to take, since it's not intrusive. We do these in other libraries.

Anyway, I wish he had done his work in collaboration with us so we could have discussed and integrated this upstream without pain.  Thanks for bringing up.

(In reply to Behdad Esfahbod from comment #7)
> Here's my take on the optimizations. First a note: KDE's use of
> FcFontMatch() with FC_FILE set is bogus and broken. Ignore that.
> 
> * 3.4.1 Rewriting FcFontMatch algorithm
> -> Valid idea. 10% is worth taking.
> 
> * 3.4.2 Value preprocessing
> -> Valid concern. It's been on my radar for years. The solution is too
> invasive for my taste. Another approach might be to add new pattern elements
> FC_FAMILY_HASH, etc, that retain the (corresponding type of) hash of the
> string. It will be equally intrusive I'm afraid. But yeah, definitely
> something to pursue and see if better implementation is possible. Definitely
> biggest waste of time in current FontConfig.  Side note: if we go all the
> way to keep hashes of family names, we might end up wanting to use a
> hashtable for FcFontMatch...
> 
> * 3.4.3 Reducing heap allocations
> -> Not clear that is measurable. But sure, is not intrusive. I'm open to.
> 
> * 3.4.4 Refactoring FcStrCaseWalkerNext
> -> Sounds fine. We might not need to triplicate it, maybe just inline that
> function.
> 
> * 3.4.5 Micro-optimization in FcCompareValueList
> -> I have a hard time believing the 38% speedup claimed. If it has
> measurable impact, sounds good to me.
> 
> * 3.4.6 Reduce FcObjectFromName call amount
> -> Not sure it's worth it; has no measurable impact.
> 
> * 3.4.7 Hint branch predictor
> -> Good to take, since it's not intrusive. We do these in other libraries.
> 
> Anyway, I wish he had done his work in collaboration with us so we could
> have discussed and integrated this upstream without pain.  Thanks for
> bringing up.

Not a problem! At present moment, we are narrowing down the rebase area - in similar of your logic to include the most important and significant optimisations. Looking forward to future work on the upstream!

On a side note, I tried to generate a .pot file so we could start working on a zh_CN translation for Fontconfig... I ended up with no .pot file at all with the command below...

cd po/ && xgettext --files-from=POTFILES.in --directory=.. --output=messages.pot

If not, when are you expecting translation works to be ready to start?

will read the whole comments later carefully but just a quick comment for:

(In reply to Mingcong Bai from comment #8)
> On a side note, I tried to generate a .pot file so we could start working on
> a zh_CN translation for Fontconfig... I ended up with no .pot file at all
> with the command below...
> 
> cd po/ && xgettext --files-from=POTFILES.in --directory=..
> --output=messages.pot
> 
> If not, when are you expecting translation works to be ready to start?

simply try to run "make update-po" on both po and po-conf. you'll get POT generated once make finished.

(In reply to Akira TAGOH from comment #9)
> will read the whole comments later carefully but just a quick comment for:
> 
> (In reply to Mingcong Bai from comment #8)
> > On a side note, I tried to generate a .pot file so we could start working on
> > a zh_CN translation for Fontconfig... I ended up with no .pot file at all
> > with the command below...
> > 
> > cd po/ && xgettext --files-from=POTFILES.in --directory=..
> > --output=messages.pot
> > 
> > If not, when are you expecting translation works to be ready to start?
> 
> simply try to run "make update-po" on both po and po-conf. you'll get POT
> generated once make finished.

That worked, thanks!

(Knowing this is not going to be too relevant to the final upstreamed implementation, but for the sake of transparency...)

We have rebased our code upon 2.12.93, the code builds and works correctly.

(In reply to Mingcong Bai from comment #11)
> (Knowing this is not going to be too relevant to the final upstreamed
> implementation, but for the sake of transparency...)
> 
> We have rebased our code upon 2.12.93, the code builds and works correctly.

Oh that definitely helps!

We are looking at a problem introduced by srb's patches on 2.12.6 and 2.12.93 that causes synaptic's UI font to be serif only under sudo on our distro. It appears that the "Match Pattern" has a way longer family list when calling through sudo. We bisected the change to the FcFontSetMatchInternal optimization commit.

I am asking Bai to get the match pattern for an unpatched version so we can find out whether it's caused by something like a change in tie-breaking behavior (weird) or by something outside of FcFontSetMatchInternal that changes the match pattern (wild). 

* * *

I put back some FC_DEBUG functionalities for FcFontSetMatchInternal so that MATCH (1) works roughly as the same as before without the best score, and MATCHV (2) shows font indexes (like before, but without set barriers) and each priority rounds and the process of bitset ellimination after each round. It can get a bit too verbose when mixed with the individual subscores and their operands...

MATCH2 (4096) is still missing. Looks like I can just paste it back.

Where's your git tree?

(In reply to Behdad Esfahbod from comment #14)
> Where's your git tree?

Sorry! I thought I've put it up here - https://github.com/AOSC-Dev/fontconfig.

The bug is now fixed in our rebased version. The strong/weak score selection in michalsrb's optimized FcFontSetMatchInternal takes the first element of pattern's valuelist's strength; it should instead decide by (matcher->strong == priority).

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/fontconfig/fontconfig/issues/103