greenlaw110 / greenscript

A tool help web developers manage javascript/css rendering, including minimize and dependence management
42 stars 24 forks source link

Cache key now MD5 hash of the resource names being combined #32

Closed tazmaniax closed 12 years ago

tazmaniax commented 12 years ago

Hi Green,

I'll have to admit that I'm not 100% understanding of all the code so I've focused on the UUID.randomUUID(). Assuming that an MD5 hash of the concatenated resource names provides a reliable hash without any collisions in the majority of cases would this provide the necessary predictable naming required by a cluster without the need of engaging a separate shared cluster wide cached index that you suggested? Or have I completely missed the boat?

I'm in the process of testing but I thought I'd get a heads up to see if it was a likely solution.

It looks like Minimizer.process(List resourceNames) is where the magic happens but I couldn't see how this was called from GreenScriptPlugin. I'm guessing on startup all the managed files are compressed and combined.

cheers, Chris

greenlaw110 commented 12 years ago

Hi Chris,

It looks a fairly good and elegant solution. The only concern is still at hash collision. But at least it should work for most cases. later on we could enable user to configure greenscript to choose between cache based or MD5 based buffer.

About Minimizer.process(List resourceNames), it's called on every request incoming, when render engine parsing the following tag:

{greenscript.(js|css) output: (all|true)}

I am sorry I don't have a good response on greenscript and morphia issues recently. working crazy on yet another static template engine named rythm. it's very close to alpha test... let me know if you want to give it a try when it's ready. would be very appreciate for that.

Thanks for you good work! Green

On Sat, Jan 28, 2012 at 8:02 AM, tazmaniax < reply@reply.github.com

wrote:

Hi Green,

I'll have to admit that I'm not 100% understanding of all the code so I've focused on the UUID.randomUUID(). Assuming that an MD5 hash of the concatenated resource names provides a reliable hash without any collisions in the majority of cases would this provide the necessary predictable naming required by a cluster without the need of engaging a separate shared cluster wide cached index that you suggested? Or have I completely missed the boat?

I'm in the process of testing but I thought I'd get a heads up to see if it was a likely solution.

It looks like Minimizer.process(List resourceNames) is where the magic happens but I couldn't see how this was called from GreenScriptPlugin. I'm guessing on startup all the managed files are compressed and combined.

cheers, Chris

You can merge this Pull Request by running:

git pull https://github.com/tazmaniax/greenscript cluster_support

Or you can view, comment on it, or merge it online at:

https://github.com/greenlaw110/greenscript/pull/32

-- Commit Summary --

  • Cache key now MD5 hash of the resource names being combined

-- File Changes --

M java/core/src/main/java/com/greenscriptool/Minimizer.java (6) M java/core/src/main/java/com/greenscriptool/utils/BufferLocator.java (9) M java/core/src/main/java/com/greenscriptool/utils/IBufferLocator.java (4) M java/play/src/play/modules/greenscript/GreenScriptPlugin.java (10)

-- Patch Links --

https://github.com/greenlaw110/greenscript/pull/32.patch https://github.com/greenlaw110/greenscript/pull/32.diff


Reply to this email directly or view it on GitHub: https://github.com/greenlaw110/greenscript/pull/32

tazmaniax commented 12 years ago

Hi Green,

Made some further changes to allow using greenscript in a cluster without a servers side cluster cache - combined resource names are included as a parameter on minimised resource URLs allowing other instances to generate minimised resource if they haven't already from page render. Possibly a problem with many resources but the change tries to reduce the size of the resource names by removing the default base path. Anyway quicker to enable then setup a cluster cache. I've tested it on Heroku and it seems to be ok. Let me know what you think.

Not too sure about your version naming standard. It looked like you were tracking the Play! version it was built against with letters for multiple releases against a single Play version. However, you seemed to have jumped head so maybe not. Any how I've made it 1.2.4b

cheers

greenlaw110 commented 12 years ago

Hi Chris,

Thank you very much. Will merge later on.

About the version number:

  1. architecture change: increase the first digit
  2. big feature change: increase the second digit
  3. small enhancement: increase the thrid digit
  4. bug fix without enhancement: increase the last alphabet.

So the version merged from your contribution will be named 1.2.7

Cheers, Green

On Sun, Jan 29, 2012 at 3:15 PM, tazmaniax < reply@reply.github.com

wrote:

Hi Green,

Made some further changes to allow using greenscript in a cluster without a servers side cluster cache - combined resource names are included as a parameter on minimised resource URLs allowing other instances to generate minimised resource if they haven't already from page render. Possibly a problem with many resources but the change tries to reduce the size of the resource names by removing the default base path. Anyway quicker to enable then setup a cluster cache. I've tested it on Heroku and it seems to be ok. Let me know what you think.

Not too sure about your version naming standard. It looked like you were tracking the Play! version it was built against with letters for multiple releases against a single Play version. However, you seemed to have jumped head so maybe not. Any how I've made it 1.2.4b

cheers


Reply to this email directly or view it on GitHub: https://github.com/greenlaw110/greenscript/pull/32#issuecomment-3706507