[
https://issues.apache.org/jira/browse/SHINDIG-103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574656#action_12574656
]
Chris Chabot commented on SHINDIG-103:
--------------------------------------
Hey Kevin,
First of all, thanks for taking the time to dive into this!
I'll fix up the copied-features issue and some other little things and
re-submit a patch. That doesn't mean other things wont be picked up and in fact
there's bound to be a few small bugs hiding in it too, but since a good few
people wanted to see this included sooner rather then later, i won't block
submitting it on having everything completely done. If we get it good enough
for initial inclusion other people can start looking at it and maybe contribute
where possible too.
Here's my reactions to the individual points:
- Don't copy the features / javascript directories.
Main reason why i went with this initially was because i saw a lot of changes
going on there (syndication, escaping of fields etc), and i didn't have the
time to track completely what this meant for the porting effort, so i copied
the branch to have a known-to-work base to work on. I'll rectify the situation
and re-submit a patch.
- In general, documentation is lacking.
- I wouldn't go out of my way to emulate Java too much in PHP
W.I.P. (work in progress), i also felt that some re-factoring would be a good
thing to do next, however as a few people suggested on the mailing list, it's
better to submit a patch now and get the ball rolling then to wait for it to be
completely Done(Tm).
Since when i began this i had no idea how the java server worked, i just jumped
in somewhere in the middle and trusted in the fact that by reading the java
base line per line a bigger picture of what it did exactly would appear;
Fortunately that worked, so i'm in a lot better position now to start making
bigger changes to it; (Zen mode) To deviate from the way you must first know
the way right? :)
Documentation is in the same line something i wanted to leave until i would
have a much clearer picture of what was what, else it would've become a byte
filler and not something sensible
- define() is horribly slow for no good reason in PHP
- include_once is also horribly slow for no good reason
Actually to my best knowledge the performance impact of include_once was fixed
somewhere in the php 5.1 era, however i do realize as i'm writing this, that
some people might still be stuck there, so i'll fix it up for that case.
- You don't need null checks when you're already doing empty() tests. empty()
will always return true for null values.
- equals methods are generally unnecessary in PHP
I went a bit overboard on some literal code porting, i'll fix that up in the
re-factoring too.
- Have you considered using __autoload
I truly love the new magic functions in php 5, however i've found in the past
that the performance of __autoload is completely horrific, i'll see how it
behaves now with php 5.2, if it improved any it is a good option, otherwise if
its still as bad as it was, i'd rather not :)
- Your use of a single main entry point is definitely the right way to go. One
huge benefit that this has (aside from obvious architectural ones) is that you
could actually just include all classes at the top level.
I'll probably put those in the 'servlets' (gadget, js, proxy) since not
everything requires the same libs, but its a good idea indeed to prevent the
include_once penalty for older installations.
- SimpleXML allows you to access attributes as $element['foo'].
I'll check it out and see if i can't spare some code there.
- You can replace Substitutions::substituteType with the following:
Great idea
- You can get the unmodified request headers in PHP using getallheaders
Haven't made my mind up yet about being apache dependent or not; The fun thing
about php is that is is a extremely diverse landscape, however i do think it's
probably unlikely (from my opinionated perspective) that someone would run PHP
on a windows box to host a gadget server, so maybe some apache dependencies is
ok to do ..
> New PHP Shindig gadget server code
> ----------------------------------
>
> Key: SHINDIG-103
> URL: https://issues.apache.org/jira/browse/SHINDIG-103
> Project: Shindig
> Issue Type: Improvement
> Components: Gadgets Server - PHP
> Reporter: Chris Chabot
> Assignee: Brian McCallister
> Attachments: php-shindig.patch
>
>
> This patch completely replaces the old code in the php directory of the
> shindig project, and replaces it with a port of the java shindig code.
> The java code used is from a checkout from February the 14th, this because
> keeping track of the changes while porting and testing would have gotten in
> the way of completing the first release in a timely fashion.
> It implements the structure and workings from the java server and includes:
> - Gadget server
> - Proxy service
> - JS service
> The maIn differences from the Java branch are:
> - Different caching engine (abstracted base class, sample file implementation
> included)
> - Different gadget rendering work flow (no multithreading in PHP)
> - No enum's or name spaced classes in php, so their top level classes now
> - Locale class to emulate the Java's Locale class
> - Somewhat servlet like event handling to stay close to the Java's structure
> Missing is the RPC feature, some custom speedups to get it closer to the
> Java's speed (it's about 40% slower right now, but that gap might be fixed
> later on), and a better logging / error reporting structure, it now only has
> a basic DEBUG flag in the config, which if set, causes it to dump debug
> backtrace's on errors.
> It's also missing Caja support, since that is Java only at the moment. Maybe
> somewhere in the future we'll have a command line version available that we
> can use.
> It also contains a copy of the features directory structure (so that it
> doesn't break on updates from the Java side of things, changes will have to
> be implemented before you can copy over the new version) and a copy of the
> sample container (minus the Caja options), so that people can test it
> directly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
