combine_javascript no longer works on trunk

[GoogleCodeExporter]

What steps will reproduce the problem?
1. compile from trunk to 0.10.0.0-1048
2.ModPagespeedRewriteLevel PassThrough
ModPagespeedEnableFilters combine_javascript,rewrite_javascript

What is the expected output? What do you see instead?
Expect to see javascript files combined. See individual rewritten javascript 
files.

What version of the product are you using (please check X-Mod-Pagespeed
header)?
0.10.0.0-1048

On what operating system?
Centos 5.7

Which version of Apache?
Apache 2.2.3

Which MPM?
Both tested

This works on 0.10.0.0-989

Original issue reported on code.google.com by tre...@rallydev.com on 5 Oct 2011 at 10:27

[GoogleCodeExporter]

I just checked the latest trunk, r1050.  It seems to work fine on 

http://localhost/mod_pagespeed_example/combine_javascript.html?ModPagespeed=on&M
odPagespeedFilters=combine_javascript

I also added ",rewrite_javascript" to match your configuration better and it 
still works.  Does it work for you using the examples directory?  Note that the 
source for this examples is available from where you configured your source 
tree, under src/install/examples.

It's also worth refreshing your browser one more time.  Recent trunk versions 
of mod_pagespeed employ an asynchronous rewriting algorithm that tries to 
improve HTML latency by doing rewrites in the background.  If the rewrites 
don't happen within a pretty tight time boundary, then the HTML is sent to the 
browser without the rewrite, but the rewrite continues in a background thread 
so we can write the final result to cache.  THis way the initial page views are 
fast, and later page views get the full benefits of the rewriters.

Finally, it's  worth cranking up the loglevel to 'debug' and seeing if anything 
interesting gets written in the error log.

Please let us know what you discover.

Original comment by jmara...@google.com on 6 Oct 2011 at 1:29

• Changed state: RequestClarification

[GoogleCodeExporter]

I updated my build to r1051 and the examples worked as expected.

However my application continues to have problems. I get these messages when 
only combine_javascript is on:

Cannot combine: url too big

I reverted back to r989 and I get the same errors, but the combine_javascript 
works. Here is an example entry:

Fetching resource 
https://test1optimized.f4tech.com/slm/js-lib,_swfobject,_2.2,_swfobject.js+js,_s
lm.js+js-lib,_json.js+js,_stopwatch.js+js,_xmlhttp.js+js,_history,_history.js+js
,_porthole.js+js,_tps.js+js,_editors.js+js,_detail.js+js-lib,_yui,_2.5.2,_build,
_yahoo-dom-event,_yahoo-dom-event.js+js-lib,_yui,_2.5.2,_build,_animation,_anima
tion-min.js+js-lib,_yui,_2.5.2,_build,_connection,_connection-min.js+js-lib,_yui
,_2.5.2,_build,_dragdrop,_dragdrop-min.js+js-lib,_yui,_2.5.2,_build,_element,_el
ement-beta-min.js+js-lib,_yui,_2.5.2,_build,_container,_container-min.js+js-lib,
_yui,_2.5.2,_build,_treeview,_treeview-min.js+js-lib,_yui,_2.2.2,_resizepanel.js
+js,_panel.js+js-lib,_yui,_2.2.2,_dd_rank.js+js,_navigation.js+js,_info.js+js,_t
abs.js+js,_plan.js+js,_legal.js+js,_help.js+js-lib,_calendar,_calendar.js+js-lib
,_calendar,_calendar-setup.js+js-lib,_calendar,_lang,_calendar-en.js+js-lib,_cal
endar,_blackoutDates.js+js,_tpsreportview.js.pagespeed.jc.C058gHUH_3.js

As you can see, it is a very long URL and this is only 1 of 13 long URLs 
generated.

I looked in the documentation and couldn't find how to turn up the debug level. 
Or is that at the Apache level?

Original comment by tre...@rallydev.com on 6 Oct 2011 at 4:57

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Using gclient sync --revision x, I used a binary dissect to get the revision 
where this behavior changed...

Build -- combine works?
989   -- yes
1053 --  no
1025 --  no
1006 --  no
1000 --  no
995  -- no
992  -- yes!!
993 -- yes
994 -- no!!

So now going to look between 993 and 994

Original comment by tre...@rallydev.com on 6 Oct 2011 at 5:02

[GoogleCodeExporter]

Thanks for tracking this down!  We should be able to take a deeper look.  This 
is the change in question.

http://code.google.com/p/modpagespeed/source/detail?r=994

We must have done something that expanded the size of the names of URLs so they 
exceeded a limit, and managed not to catch it in a unit-test.

Original comment by jmara...@google.com on 6 Oct 2011 at 5:06

• Changed state: Accepted

[GoogleCodeExporter]

Actually it's quite possible that this was related to Maksim's threading 
change, rather than Matt's naming change, but because they got committed 
together we don't know which one broke the testcase yet.

Original comment by jmara...@google.com on 6 Oct 2011 at 5:09

[GoogleCodeExporter]

Yeah, so no. There is a lot of changes here where we add threading and change 
the url naming. Looks like a strong candidate, but I'm no C/C++ programmer and 
I'll leave that to the experts.

Original comment by tre...@rallydev.com on 6 Oct 2011 at 5:12

[GoogleCodeExporter]

Is there any public URL with the document that triggers this bug? 

Original comment by morlov...@google.com on 6 Oct 2011 at 6:27

[GoogleCodeExporter]

Sorry, not yet.

Original comment by tre...@rallydev.com on 6 Oct 2011 at 7:14

[GoogleCodeExporter]

Hmm. Looking at your URLs a bit, looks like there are multiple directories 
involved. 
I think you have an slm/ with a js-lib/ and js/ under it? Which directory is 
the page in?

Original comment by morlov...@google.com on 6 Oct 2011 at 7:22

[GoogleCodeExporter]

The page is at /slm/

There is a lot of javascript under js and js-lib and we tend to preload it all 
on the login page. Currently we use packtag, but would like to get away from it

Original comment by tre...@rallydev.com on 6 Oct 2011 at 8:01

[GoogleCodeExporter]

Update.. and it doesn't help solve the issue. We have over 100 javascript 
references (don't get me started here), so I missed it, but a few of the 
resources do get combined! With build 989, all get combined down to 13 files, 
but with anything after 993, there are about 7 combined files, and the rest are 
left as individual files (but rewritten).

Original comment by tre...@rallydev.com on 6 Oct 2011 at 9:52

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Yes, the debug level is just the Apache LogLevel. One question with respect to 
that: do you by any chance get messages in log like " failed to stat (code=36 
File name too long)"?

Original comment by morlov...@google.com on 7 Oct 2011 at 4:47

[GoogleCodeExporter]

Yes I do!

/var/www/mod_pagespeedcache/https,2C3A,2C_,2C_tes...ayout,2C_Header.js,2CMjm.9eX
Jd9N_Wc,-:0: failed to stat (code=36 File name too long)

I truncated the line, but it looks to be an about 8020 characters in the entire 
path andabout 7,991 characters in the filename. 

Original comment by tre...@rallydev.com on 7 Oct 2011 at 5:22

[GoogleCodeExporter]

Original comment by morlov...@google.com on 10 Oct 2011 at 2:04

• Changed state: Started

[GoogleCodeExporter]

r1064 should hopefully resolve this.

Original comment by morlov...@google.com on 11 Oct 2011 at 2:59

[GoogleCodeExporter]

You rock! This works like a charm. Thank you.

Original comment by tre...@rallydev.com on 11 Oct 2011 at 3:52

[GoogleCodeExporter]

Original comment by sligocki@google.com on 11 Oct 2011 at 4:24

• Changed state: Fixed