Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible Bug: Adding many 'font' options delays preview refresh #48

Closed
ardalann opened this issue Mar 10, 2014 · 17 comments
Closed

Possible Bug: Adding many 'font' options delays preview refresh #48

ardalann opened this issue Mar 10, 2014 · 17 comments
Labels

Comments

@ardalann
Copy link
Contributor

I don't know if it's a bug within TF or not, but it's really annoying.

I added 7 'font' options (6 of which are for h1 to h6). I also have 8 other 'font' options in other sections.

I noticed before, that when I add a 'font' option, it adds a delay for reloading the preview after editing an option that doesn't have 'livepreview'. (not necessarily a font option)
It's not because of low internet speed or server speed. The request to the server is made AFTER this delay. After I change an option, I also click on another option to trigger 'blur' and 'change' js events.

Now that I added 7 'font' options all at once, I'm noticing a huge delay before reloading the preview iframe.
With 8 'font' options it has about 8sec delay, and with 15 it has about 17sec delay.

Any idea what's causing this? Anyone else having the same issue?

@bfintal
Copy link
Contributor

bfintal commented Mar 10, 2014

I tried adding a bunch of font options, checked also the developer console in Chrome. I can see that right after a change was made, a network request was immediately sent. So I think the cause for the slow down might be that multiple different fonts are being loaded which can be slow. The amount of css and font files may become large and can slow down the preview.

Do they all have the same font families?

@ardalann
Copy link
Contributor Author

@bfintal You're right. I just used developer console Network tab and noticed the request is immediately sent, but the response takes 20 seconds to be sent. All my font pickers have Open Sans selected, so it's not the font. (Chrome console also says loading fonts and everything is started after that 18-20 seconds.

I think it's related to the thing I mentioned in #44 . (where there were many errors for a few font fields). It could be so many connections made to the DB, or so much process, that makes it very slow when there are many font fields. And it's only in customizer preview, not in frontend or admin pages.
Maybe it's related to the hooks problem #50 . I'll see if this problem persists after fixing #50 .

@bfintal
Copy link
Contributor

bfintal commented Mar 10, 2014

I don't think it's related though to #44, not sure about #50. I'm doing #50 right now so we will see in a bit whether that's it.

Maybe it's the loading of the new styles also in the head tag, although we are doing things the way the Codex recommends it. Perhaps WP is causing the delay.

Would you be able to investigate what it's doing in the 20 second response?

@ghost
Copy link

ghost commented Mar 11, 2014

Are you including the CSS files from Google asynchronously? That should speed up the rendering of the page a lot. The web fonts are displayed when they are completely loaded, without blocking any other request. Take a look at this: https://github.com/typekit/webfontloader#google

Also, you can define just a subset of characters to download. That will speed up even more the download because you are not getting any unnecesary glyph, you just get the characters you need.

@bfintal
Copy link
Contributor

bfintal commented Mar 11, 2014

@davidossahdez Nope, we're just including Google fonts the normal way via wp_enqueue_style Your suggestion can be done, although that isn't the problem with what @ardalann is experiencing since the refresh of the preview pane is getting delayed and not the contents. (Correct me if I'm wrong there)

@bfintal
Copy link
Contributor

bfintal commented Mar 11, 2014

@ardalann Just closed #50, can you give that a try? Not too sure it would have an impact. Unfortunately I can't replicate what you're experiencing on my end :(

@bfintal bfintal added the bug label Mar 11, 2014
@ardalann
Copy link
Contributor Author

It's definitely server-side.
These are my findings so far:

  • Maybe it's not exactly because of font settings, but each font setting adds a lot of settings and increases the number of settings significantly. Right now my customizer preview reload takes 15 seconds (PHP process time). When I remove all the font settings or all the other settings, it's reduced to 2-4 seconds.
  • I put a timer on different sections of code. This is the timer:
$GLOBALS['time_start'] = microtime(true);
...codes...
$time = microtime(true) - $GLOBALS['time_start']; echo $time;

At the moment, I've reached this:

$GLOBALS['time_start'] = microtime(true);
wp_head();
$time = microtime(true) - $GLOBALS['time_start']; echo $time; 

Which echoes a number like: 13.9210012300

function wp_head() {
    do_action('wp_head');
}

So it's definitely coming from 'wp_head' action.
I'm going to use the timer in WP core processes to find the exact source.

@ardalann
Copy link
Contributor Author

It's all coming from class-theme-customizer-section.php printPreviewCSS() function.
I have 10 customizer sections, and this function creates all the CSS codes 10 TIMES.
Each time it tries to generate CSS codes, it takes about 1.4 seconds. Multiplied by 10 sections = the delay I was talking about.
I see @bfintal has thought of it, that's why he used '$this->generatedHeadCSSPreview'. But this property is set for each section and is not working as intended.

@ardalann
Copy link
Contributor Author

There's also another issue which is probably why the "font" option had a huge impact on speed. The same CSS code for each "font" option is generated and printed out 15 times.
So basically, when I added 7 font options, it was like I added 15x7 = 105 options. That's why the process time increased significantly.
It needs to be fixed too.
If I could use Github software I'd have done something myself.

@bfintal
Copy link
Contributor

bfintal commented Mar 12, 2014

Wow, thanks for the findings! It shouldn't print that out so many times

@bfintal
Copy link
Contributor

bfintal commented Mar 12, 2014

Okay so the problem was that $generatedHeadCSSPreview was declared as private, it should have been private static

@ardalann
Copy link
Contributor Author

@bfintal Thanks ! The first bug is fixed now. How about the other one? Font settings are printed out 15 times

@bfintal
Copy link
Contributor

bfintal commented Mar 12, 2014

What's the second one? I thought everything was being slowed down because of the multiple printing of css?

@ardalann
Copy link
Contributor Author

@bfintal Yeah that was mainly because of that. But still 1.4seconds for generating the CSS seems a bit too much. One reason for it is that every font CSS is generated and printed out 15 times. Example:

.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}
.page-content{font-family:"Open Sans";color:#333;font-size:13px;font-weight:normal;font-style:normal;line-height:1.5em;letter-spacing:normal;text-transform:none;font-variant:normal;text-shadow:none;}

@ardalann
Copy link
Contributor Author

I just found out the above bug ^ (font settings CSS being repeated 15 times) is not only in the customizer, but also in the CSS file generated

@bfintal bfintal reopened this Mar 12, 2014
@ardalann
Copy link
Contributor Author

Awesome! It's fixed now.
Is it 12pm there? Go get some sleep man! It's 10AM here 😄

@bfintal
Copy link
Contributor

bfintal commented Mar 12, 2014

Haha, yes! Was just about to :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants