Some change of improving the loading speed in web page

[quguangfan, 2010-04-19 06:47:50]

Hi,

I found that when we select the route or begin times of one trip in the web
page, the loading speed is very slowly.

So there are two modifications on loading speed in the web page.

Please help review.
Thanks.

[wLiu.sjtu, 2010-04-19 07:02:32]

http://codereview.appspot.com/904045/diff/1/2
File python/transitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/1/2#newcode1108
python/transitfeed_editor.py:1108: trip.pattern_id = shape_hash
Seems it has no relationship with CreateIndex, right? I noticed that
trip.pattern_id are referenced in a few other places, so without your changes
here, where does trip.pattern_id come from?

[quguangfan, 2010-04-19 07:25:16]

Please help review.

http://codereview.appspot.com/904045/diff/1/2
File python/transitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/1/2#newcode1108
python/transitfeed_editor.py:1108: trip.pattern_id = shape_hash
On 2010/04/19 07:02:32, wLiu.sjtu wrote:
> Seems it has no relationship with CreateIndex, right? I noticed that
> trip.pattern_id are referenced in a few other places, so without your changes
> here, where does trip.pattern_id come from?

It has no relationship with CreateIndex().
The trip.pattern_id is generate at LINE 1822 in _transitfeed.py

#elif name == 'pattern_id':
#      if '_pattern_id' not in self.__dict__:
#        self.__dict__['_pattern_id'] = hash(self.GetPattern())
#      return self.__dict__['_pattern_id']

And It references in file schedule_viewer.py.

So if we don't do this and when we select a route the first time, the pattern_id
will be generated at the same time, and it will load stop_times listings from
DB, create Stops and hash the stops which are cost some time.

So we extract this at here, it can reduce so much time when we select a route or
a trip on the web page.

[wLiu.sjtu, 2010-04-19 07:36:48]

http://codereview.appspot.com/904045/diff/1/2
File python/transitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/1/2#newcode1108
python/transitfeed_editor.py:1108: trip.pattern_id = shape_hash
Great, please add a summary to your comments.

Also add a unittest.

On 2010/04/19 07:25:16, quguangfan wrote:

[calidion, 2010-04-19 07:39:06]

http://codereview.appspot.com/904045/diff/1/2
File python/transitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/1/2#newcode1108
python/transitfeed_editor.py:1108: trip.pattern_id = shape_hash
the shape hash is different from pattern id.
this change may break the uniqueness of the shape hash from each others.

On 2010/04/19 07:25:16, quguangfan wrote:
> On 2010/04/19 07:02:32, wLiu.sjtu wrote:
> > Seems it has no relationship with CreateIndex, right? I noticed that
> > trip.pattern_id are referenced in a few other places, so without your
changes
> > here, where does trip.pattern_id come from?
> 
> It has no relationship with CreateIndex().
> The trip.pattern_id is generate at LINE 1822 in _transitfeed.py
> 
> #elif name == 'pattern_id':
> #      if '_pattern_id' not in self.__dict__:
> #        self.__dict__['_pattern_id'] = hash(self.GetPattern())
> #      return self.__dict__['_pattern_id']
> 
> And It references in file schedule_viewer.py.
> 
> So if we don't do this and when we select a route the first time, the
pattern_id
> will be generated at the same time, and it will load stop_times listings from
> DB, create Stops and hash the stops which are cost some time.
> 
> So we extract this at here, it can reduce so much time when we select a route
or
> a trip on the web page.

[quguangfan, 2010-04-19 13:50:22]

http://codereview.appspot.com/904045/diff/1/2
File python/transitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/1/2#newcode1108
python/transitfeed_editor.py:1108: trip.pattern_id = shape_hash
On 2010/04/19 07:39:06, calidion wrote:
> the shape hash is different from pattern id.
> this change may break the uniqueness of the shape hash from each others.
> 
> On 2010/04/19 07:25:16, quguangfan wrote:
> > On 2010/04/19 07:02:32, wLiu.sjtu wrote:
> > > Seems it has no relationship with CreateIndex, right? I noticed that
> > > trip.pattern_id are referenced in a few other places, so without your
> changes
> > > here, where does trip.pattern_id come from?
> > 
> > It has no relationship with CreateIndex().
> > The trip.pattern_id is generate at LINE 1822 in _transitfeed.py
> > 
> > #elif name == 'pattern_id':
> > #      if '_pattern_id' not in self.__dict__:
> > #        self.__dict__['_pattern_id'] = hash(self.GetPattern())
> > #      return self.__dict__['_pattern_id']
> > 
> > And It references in file schedule_viewer.py.
> > 
> > So if we don't do this and when we select a route the first time, the
> pattern_id
> > will be generated at the same time, and it will load stop_times listings
from
> > DB, create Stops and hash the stops which are cost some time.
> > 
> > So we extract this at here, it can reduce so much time when we select a
route
> or
> > a trip on the web page.
> 
Although they are different values, they are generated by the same principle.
One is md5(stop_ids + route_id) and the other is hash(tuple(stops)). I found
they are the same unique.

Would you please provide some examples if you find the two values don't follow
the law as the above.

[calidion, 2010-04-20 05:51:13]

they are not.
if the shape ids do not exist and the trips are of the same stop sequence
from different routes, the trips will get the same hash and md5 with route
id will not。

2010/4/19 <quguangfan@gmail.com>

>
> http://codereview.appspot.com/904045/diff/1/2
> File python/transitfeed_editor.py (right):
>
> http://codereview.appspot.com/904045/diff/1/2#newcode1108
> python/transitfeed_editor.py:1108: trip.pattern_id = shape_hash
> On 2010/04/19 07:39:06, calidion wrote:
>
>> the shape hash is different from pattern id.
>> this change may break the uniqueness of the shape hash from each
>>
> others.
>
>  On 2010/04/19 07:25:16, quguangfan wrote:
>> > On 2010/04/19 07:02:32, wLiu.sjtu wrote:
>> > > Seems it has no relationship with CreateIndex, right? I noticed
>>
> that
>
>> > > trip.pattern_id are referenced in a few other places, so without
>>
> your
>
>> changes
>> > > here, where does trip.pattern_id come from?
>> >
>> > It has no relationship with CreateIndex().
>> > The trip.pattern_id is generate at LINE 1822 in _transitfeed.py
>> >
>> > #elif name == 'pattern_id':
>> > #      if '_pattern_id' not in self.__dict__:
>> > #        self.__dict__['_pattern_id'] = hash(self.GetPattern())
>> > #      return self.__dict__['_pattern_id']
>> >
>> > And It references in file schedule_viewer.py.
>> >
>> > So if we don't do this and when we select a route the first time,
>>
> the
>
>> pattern_id
>> > will be generated at the same time, and it will load stop_times
>>
> listings from
>
>> > DB, create Stops and hash the stops which are cost some time.
>> >
>> > So we extract this at here, it can reduce so much time when we
>>
> select a route
>
>> or
>> > a trip on the web page.
>>
>
> Although they are different values, they are generated by the same
> principle. One is md5(stop_ids + route_id) and the other is
> hash(tuple(stops)). I found they are the same unique.
>
> Would you please provide some examples if you find the two values don't
> follow the law as the above.
>
>
> http://codereview.appspot.com/904045/show
>

[quguangfan, 2010-04-21 01:37:40]

Changed with calidion said and add the test case.
Please help review. Thanks.

[wLiu.sjtu, 2010-04-21 02:33:05]

http://codereview.appspot.com/904045/diff/10001/11001
File python/test/testtransitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/10001/11001#newcode250
python/test/testtransitfeed_editor.py:250: self.assertNotEqual(pattern_id1,
pattern_id2)
Please add comments on why pattern_id1 != pattern_id2, and the same for the next
assertEqual(). Besides, is it necessary to add agency, weekperiod etc. for this
test? If not, you can remove them and simplify the test.

[calidion, 2010-04-21 03:15:04]

 LGTM for transitfeed_editor.py

and it seems pattern ids would also be better generated with route id for
its uniqueness if we are going to store the pattern id into the database for
acceleration and without the interferences between routes.

2010/4/21 <quguangfan@gmail.com>

> Changed with calidion said and add the test case.
> Please help review. Thanks.
>
>
> http://codereview.appspot.com/904045/show
>

[quguangfan, 2010-04-21 03:16:56]

Changed the testcase.

Thanks.

http://codereview.appspot.com/904045/diff/10001/11001
File python/test/testtransitfeed_editor.py (right):

http://codereview.appspot.com/904045/diff/10001/11001#newcode250
python/test/testtransitfeed_editor.py:250: self.assertNotEqual(pattern_id1,
pattern_id2)
On 2010/04/21 02:33:05, wLiu.sjtu wrote:
> Please add comments on why pattern_id1 != pattern_id2, and the same for the
next
> assertEqual(). Besides, is it necessary to add agency, weekperiod etc. for
this
> test? If not, you can remove them and simplify the test.

Agency and service_period are necessary because schedule.AddRouteObject will
check if the agency_id is in schedule's agency_list. And the same to
service_period.

[wliu.sjtu_gmail.com, 2010-04-21 03:37:51]

LGTM

On Wed, Apr 21, 2010 at 11:16 AM, <quguangfan@gmail.com> wrote:

> Changed the testcase.
>
> Thanks.
>
>
> http://codereview.appspot.com/904045/diff/10001/11001
> File python/test/testtransitfeed_editor.py (right):
>
> http://codereview.appspot.com/904045/diff/10001/11001#newcode250
> python/test/testtransitfeed_editor.py:250:
> self.assertNotEqual(pattern_id1, pattern_id2)
> On 2010/04/21 02:33:05, wLiu.sjtu wrote:
>
>> Please add comments on why pattern_id1 != pattern_id2, and the same
>>
> for the next
>
>> assertEqual(). Besides, is it necessary to add agency, weekperiod etc.
>>
> for this
>
>> test? If not, you can remove them and simplify the test.
>>
>
> Agency and service_period are necessary because schedule.AddRouteObject
> will check if the agency_id is in schedule's agency_list. And the same
> to service_period.
>
>
> http://codereview.appspot.com/904045/show
>