[mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source

Sat Aug 13 22:22:50 EDT 2011

as we aren't exactly re-inventing the wheel here, how do other data
abstraction layers handle this?

as i said before, we already can deteremine, if there is potentially null
values from column metadata.

When rendering, data is selected by spatial queries, how can null geometries
be returned when rendering by BBOX ?

z

On Fri, Aug 12, 2011 at 3:37 AM, Traian Stanev
<traian.stanev at autodesk.com> wrote:
>
> Null is common. There are many sparse data sets where just a few rows have a value set and all the other rows have a null value.
>
> It might make sense to remove the null check only for the geometry value when rendering, which is almost never null, but I remember there was a data set with lots of null geometries as well.
>
> Another thing to consider -- even if nulls happen only 10% of the time -- if the patch makes it 2x faster for 90% of data sets, but something like 10x (or worse) slower for 10% of data sets, then the overall render time would still be the same, so there would be no overall improvement, and what's worse, performance would be far less predictable (there would be huge variance in render times).
>
> Traian
>
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Buddy Hu
> Sent: Thursday, August 11, 2011 9:25 AM
> To: MapGuide Internals Mail List
> Subject: RE: [mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source
>
> Hi Trevor,
>
> I agree with you and Traian that we should push down the IsNull to FDO providers.
> For this patch, we replace the IsNull check with try-catch, doesn't make thing worse. As Traian said, it makes Server faster for most data sets. Although for data sets with lot of null values, it makes MG slower.
> Are you sure the Null values are common? If the Null values are more than non-Null values, I think this patch doesn't help to improve the MG performance but hurt.
>
> Regards,
> Buddy
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Trevor Wekel
> Sent: Wednesday, August 10, 2011 10:40 PM
> To: MapGuide Internals Mail List
> Subject: RE: [mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source
>
> I agree with Traian.  Faster IsNull processing should be pushed down to FDO.  I assume there is a ReadNext() call made to Fdo before these methods are called.  Perhaps the entire record including "null" state for each property could be cached during the ReadNext() call?  Nulls  are fairly common, especially in databases, so this exception will get hit regularly.
>
> The other issue is memory leaks in FDO and MapGuide.  Since nulls are common, the stack unwind should be checked for all providers and all "Get" calls to ensure that no memory leaks will occur.
>
> Regards,
> Trevor
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Martin Morrison
> Sent: August 10, 2011 5:57 AM
> To: MapGuide Internals Mail List
> Subject: RE: [mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source
>
> The only gotcha is NULL is a valid value.  MapGuide/FDO should handle it better.
>
> Martin Morrison
> Application Engineer
> Engineering Design Systems, Inc.
> 3780 Peters Creek Rd Ext SW
> Roanoke, VA  24018
> 540.345.1410
> gis.edsi.com
>
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Zac Spitzer
> Sent: Wednesday, August 10, 2011 4:12 AM
> To: MapGuide Internals Mail List
> Subject: Re: [mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source
>
> as most, if not all have databases have a not null flag per column?
>
> that metadata could help with handling nullable columns?
>
> it does get tricky with the different handling of empty strings
>
> On Wed, Aug 10, 2011 at 4:43 PM, Traian Stanev <traian.stanev at autodesk.com> wrote:
>>
>> The initial implementation for the rendering code indeed used optimistic null handling via try-catch instead of checking for IsNull, for performance reasons. For most data sets, not checking for null is a lot faster, because in FDO checking for IsNull gets the property value a second time. However, there were some sample data sets with lots of null values, which incurred a big overhead because of the number of catches that happened -- so someone added the IsNull. For such data, having the IsNull was a lot faster due to the inherent slowness of catching.
>>
>> The facts have not changed since then -- removing the null check will make things faster for data sets with no null values but slower for data sets with lots of null values.
>>
>> Really, MapGuide should not have to work around FDO being slow. There must be some way to optimize FDO providers where this is slow to cache the value after checking it for null and then quickly return it knowing that the next property value requested will likely be for the property that was previously checked for null.
>>
>> Traian
>>
>>
>>
>>
>>
>> -----Original Message-----
>> From: mapguide-internals-bounces at lists.osgeo.org
>> [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Zac
>> Spitzer
>> Sent: Wednesday, August 10, 2011 12:36 AM
>> To: MapGuide Internals Mail List
>> Subject: Re: [mapguide-internals] Code Reveiw request: Remove IsNull
>> check when get property from feature source
>>
>> that old thread was titled [mapguide-internals] Exception thrown on null values?
>>
>> here is some of it, dates back to 08, which was pre-nabble days?
>>
>> http://osgeo-org.1803224.n2.nabble.com/RE-mapguide-internals-Exception
>> -thrown-on-null-values-td2051195.html
>>
>> from my *limited* understanding, this patch catches any exceptions and
>> then gracefully handles them which should be a big win
>>
>> z
>>
>>
>> On Wed, Aug 10, 2011 at 2:28 PM, Jason Birch <jason at jasonbirch.com> wrote:
>>> Wasn't the isnull check initially put in place to deal with stability
>>> issues (something to do with FDO exceptions not unwinding properly or
>>> something?)
>>>  It was a long time ago, but it sounds pretty familiar.
>>>
>>> On 1 August 2011 01:15, Buddy Hu wrote:
>>>
>>>> Hi Lists,
>>>>
>>>> Please add your comments for this patch:
>>>> http://trac.osgeo.org/mapguide/ticket/1766
>>>>
>>>> Regards,
>>>> Buddy
>>>> _______________________________________________
>>>> mapguide-internals mailing list
>>>> mapguide-internals at lists.osgeo.org
>>>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>>>>
>>> _______________________________________________
>>> mapguide-internals mailing list
>>> mapguide-internals at lists.osgeo.org
>>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>>>
>>
>>
>>
>> --
>> Zac Spitzer
>> Solution Architect / Director
>> Ennoble Consultancy Australia
>> http://www.ennoble.com.au
>> http://zacster.blogspot.com
>> +61 405 847 168
>> _______________________________________________
>> mapguide-internals mailing list
>> mapguide-internals at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>>
>> _______________________________________________
>> mapguide-internals mailing list
>> mapguide-internals at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>>
>>
>
>
>
> --
> Zac Spitzer
> Solution Architect / Director
> Ennoble Consultancy Australia
> http://www.ennoble.com.au
> http://zacster.blogspot.com
> +61 405 847 168
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>
>
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>
>

-- 
Zac Spitzer
Solution Architect / Director
Ennoble Consultancy Australia
http://www.ennoble.com.au
http://zacster.blogspot.com
+61 405 847 168