Jump to content

Find most efficient solution for storing multiple values in a field


gebeer
 Share

Recommended Posts

looking at Timestamp.php method get:

	/**
	 * Retrieve a value from the timestamp: date, location or notes
	 *
	 */
	public function get($key) {
		$value = parent::get($key);

		// if the page's output formatting is on, then we'll return formatted values       <<< !!!
		if($this->page && $this->page->of()) {

			if($key == 'date') {
				// format a unix timestamp to a date string
				$value = date(self::dateFormat, $value);

			}
...
  • Like 1
Link to comment
Share on other sites

@horst

I see what you mean. But in my code I already turn off output formatting. S I don't quite understand why a formatted value is being returned.

        $editpage->of(false);
        foreach($adform as $field) {
            if(in_array($field->name, $ignorefields)) continue;
            $editpage->set($field->name, $field->value);
        }
        $editpage->title = $editpageTitle->value;
        $editpage->name = "ad-".$sanitizer->pageName($editpage->title)."-".$uID;

        $deletetimer = Debug::timer();
        //delete all old timestamps for that page before saving new ones
        $numberDeleted = iterator_count($editpage->ad_publish_at);
        foreach ($editpage->ad_publish_at as $t) {
            $editpage->ad_publish_at->remove($t);
        }
        $deletetimer = Debug::timer($deletetimer);

        $runtimer = Debug::timer();
        $startTime = $adform->get("ad_publish_up")->value;
        $endTime = $adform->get("ad_publish_down")->value;
        $interval = $adFrequency->value->value;
        if ($interval == "0") {
            $numberCreated = 1;
            $timestamp = new Timestamp();//this is the Class Timestamp found in Timestamp.php. Included via FieldtypeTimestamps.
            $timestamp->date = $startTime; // note stored in column 'data' in the db of the Field
            $editpage->ad_publish_at->add($timestamp);
        } else {
            $times = getRecurrences($startTime,$endTime,$interval);
            $numberCreated = iterator_count($times);

            foreach ($times as $t) {
                $t = $t->getTimestamp();
                $timestamp = new Timestamp();//this is the Class Timestamp found in Timestamp.php. Included via FieldtypeTimestamps.
                $timestamp->date = $t; // note stored in column 'data' in the db of the Field
                $editpage->ad_publish_at->add($timestamp);
             }
        }
        $runtimer = Debug::timer($runtimer);

        $editpage->save();
        $editpage->of(true);

  • Like 1
Link to comment
Share on other sites

Actually if you look at the code in Timestamp.php, lines 40-47, that is where the conversation from string to integer happens so you don't have to do this in your foreach; Providing a date string is enough  :-)

if($key == 'date') {
    // convert date string to unix timestamp
    if($value && !ctype_digit("$value")) $value = strtotime($value); 	
    // sanitized date value is always an integer
    $value = (int) $value;
}

See also the comment in FieldtypeTimestamps.module lines 140-1

// note that sanitization of individual fields within a given timestamp is already 

// performed by the Timestamp::set() method, so we don't need to do anything else here.

Also never got the errors you were getting. But I suppose your implementation of the code is different so I may be way off here...

Btw, about your earlier questions on how stuff was getting deleted in the backend? The module is using trackChanges. You can read more here about the method.

Edit:

Aaah, I think I now see why you were getting that PHP notice. As pointed out in the SO link you gave, date() expects the second parameter to be an integer. In the case of InputfieldTimestamps.module, it's getting its values from the database. Those values were already saved as integers so no need for strtotime at that point. In your case, your values are coming from your frontend form as strings so they needed to be parsed into timestamps. At least I think this is what was happening. Anyway, you sorted it out but just thought to clarify this. Or maybe, I am not getting what's going on....I am having one of those slow days...again...sigh...

Edited by kongondo
  • Like 2
Link to comment
Share on other sites

@kongondo

In my foreach, the values are converted to timestamps before I send them to the Timestamps module (note the 2nd line here)

foreach ($times as $t) {
                $t = $t->getTimestamp();
                $timestamp = new Timestamp();//this is the Class Timestamp found in Timestamp.php. Included via FieldtypeTimestamps.
                $timestamp->date = $t; // note stored in column 'data' in the db of the Field
                $editpage->ad_publish_at->add($timestamp);
             }

That's why I don't understand that PHP Notice. But my solution to convert the value again with strtotime seems to be working fine and everything behaves as expected. So no need to worry about it any further.

  • Like 1
Link to comment
Share on other sites

OK, this is solved now.

The most efficient and performant method for storing multiple (even thousands) of timestamps with a page definitely is a custom input field type that stores values in an extra table in the DB which makes it really fast.

Thanks to all who contributed, especially kongondo who was so kind to supply a customized input field Timestamps module derived from the input field events module.

I can now save about 3000 timestamps for a page within 2 seconds which is really fast compared to other methods that I have tried.

Also searching through those timestamps is pretty fast, even when it comes to hundreds of pages that need to be searched for a timestamp value.

PW really is great tool and the detailed help and instructions here in the forum is absolutely awesome. I also learned a lot about PHP and the inner workings of PW during the process.

Thanks again!

  • Like 6
Link to comment
Share on other sites

@kongondo

My implementation is as follows.

1. installed the inputfield Timestamps module that you supplied.

2. created a field "ad_publish_at" of type timestamp and add it to my advertisement template

3. render the form on the frontend with code based on Soma's gist.

4. use a custom function to find recurrences from start and end dates in my advertisement template. The function uses the barely documented PHP DatePeriod class

function getRecurrences($startTime,$endTime,$interval) {
    $format = 'Y-m-d H:i';
    $start = new DateTime();
    $end = new DateTime();
    $start->setTimestamp($startTime);
    $end->setTimestamp($endTime);
    $end = $end->modify( '+1 minute' ); 

    $duration = 'PT' . $interval . 'M'; //where $interval is 15, 30 or 60
    $interval = new DateInterval($duration);
    $daterange = new DatePeriod($start, $interval ,$end);

    return $daterange;
}

5. $daterange contains an array of DateTime objects. I loop through them, convert each into a timestamp and save that to my timestamps field

            $times = getRecurrences($startTime,$endTime,$interval); //this is the daterange array with DateTime objects

            foreach ($times as $t) {
                $t = $t->getTimestamp();
                $timestamp = new Timestamp();//this is the Class Timestamp found in Timestamp.php. Included via FieldtypeTimestamps.
                $timestamp->date = $t; // note stored in column 'data' in the db of the Field
                $editpage->ad_publish_at->add($timestamp);
             }

6. when a user edits an advertisement and changes dates, I have to manually delete all previously saved timestamps, before I add the new ones with step 5 above. For deleting the "old" timestamps I use this

        foreach ($editpage->ad_publish_at as $t) { //ad_publish_at is my timestamps field
            $editpage->ad_publish_at->remove($t);
        }

If you have any further questions on my implementation, please ask.

  • Like 3
Link to comment
Share on other sites

I'm wondering, too if this could be automated.

When I just add new timestamps, the old ones are still there. That's why I'm deleting them before I add new ones. Deleting takes around 1.6 seconds for 3000 timestamps. So I can live with that.

But if there is a way to make this faster, I would be very interested.

Link to comment
Share on other sites

The automation could start right from here...detecting when a user edits an advert:

when a user edits an advertisement and changes dates


But I don't know what sort of traffic you will on the site, whether these users are editors, etc...

Link to comment
Share on other sites

I implemented it so that the deletion of old timestamps happens only after the user submits the form and before the new timestamps get created and saved.

For my scenario this is sufficient and I don't need other automation options.

I'm just wondering whether there might be a faster method to delete timestamps other than my foreach loop in step #6 above (which is pretty fast already anyways :) ).

Link to comment
Share on other sites

@adrian

Just tried your code and compared time for both methods.

Deletion of 3073 timestamps.

using

$editpage->ad_publish_at->removeAll();

0.2848 seconds

using

foreach ($editpage->ad_publish_at as $t) { //ad_publish_at is my timestamps field
            $editpage->ad_publish_at->remove($t);
        }

0.7537 :)

So removeAll() is significantly faster and speeds up my application. Thank you!

Cheers

Gerhard

  • Like 4
Link to comment
Share on other sites

Pretty happy to see how you guys have sorted this out for the OP :)

I'd just like to point out that

foreach ($editpage->ad_publish_at as $t) { //ad_publish_at is my timestamps field
  $editpage->ad_publish_at->remove($t);
}

...will be a lot slower than WireArray's removeAll(). The reason for this is that the implementation above causes a scan in the array (you are asking an object to be removed from the array). So while there is a foreach loop inside removeAll(), it's different, because it removes based on the keys of the array (which is faster).

Changing the original implementation to

foreach ($editpage->ad_publish_at as $k=>$t) { //ad_publish_at is my timestamps field
  $editpage->ad_publish_at->remove($k);
}

...would have the exact same speed as removeAll(). But obviously if you don't need to do anything else to the object, it makes more sense to just use removeAll().

Edit: While I was writing you obviously confirmed this with your benchmark ;)

Edited by sforsman
  • Like 6
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...