jonaswouters / XhprofBundle

XHProf bundle for Symfony 2
210 stars 49 forks source link

Bypass Doctrine in favor of Xhgui serialization #74

Closed aheinz-sg closed 8 years ago

aheinz-sg commented 9 years ago

This is still a work in progress, but I wanted to get it up for review. I have a few problems with it as it currently stands:

  1. The hardcoded change to Resources/config/services.xml is inappropriate, but I haven't found the syntax to get it to pick up my parameter, i.e. <argument type="service" id="%jns_xhprof.manager_registry%" on-invalid="null" />, which doesn't work.
  2. It doesn't output fully Xhgui-compatible results. This causes Xhgui to error (warning) out with "Undefined index: meta" when I attempt to view the run list. Currently experimenting with dialing down my error_reporting.
  3. I originally implemented this with less copy/paste. It was simple enough to pull most of the entity into a model class, then have both entity and document inherit the model, which didn't work. None of the properties from the mapped superclass were picked up and it was saving to mongo documents with only an id.
aheinz-sg commented 9 years ago

Ok! This second effort is quite a bit better and actually works! I made the decision to use the Xhgui_Saver_Mongo rather than try to build up an equivalent Doctrine representation. The dependency injection could be cleaner (always hate to implement ContainerAwareInterface), but at least it's configurable.

jonaswouters commented 9 years ago

@aheinz-sg The support for the original implementation is removed in favour of mongo? Any way you can make the old version be the default and your implementation an option? Backwards compatibility etc...

aheinz-sg commented 9 years ago

@jonaswouters I had exactly this same argument with myself when I wrote it. The factors that swayed me to remove it instead of leaving it around:

  1. Xhgui only supports Mongo.
  2. When I cloned Jns\Bundle\XhprofBundle\Entity into a Document to get it into Mongo, the serialized document was incompatible with Xhgui.
  3. The configuration switch was named enable_xhgui.

So what I saw was a piece of code that purported to integrate with Xhgui, but didn't, so I didn't see any reason to keep any of it around. If you think that general purpose saving to a doctrine entity is useful, I can't argue with that, but then I think that enable_xhgui is a badly named configuration switch and should be renamed... so I'm BC breaking no matter what I do.

aheinz-sg commented 9 years ago

@jonaswouters Let me know which direction you want to go with this. I have an idea on how to make Xhgui a drop-in replacement for the stock Xhprof run viewer, so I may try to implement that as well when I make whatever changes are necessary to get this pulled.

aheinz-sg commented 8 years ago

As requested, I restored the original Entity serialization. My additions are wired to enable_xhgui_new instead, including working links into Xhgui from the Symfony toolbar.

aheinz-sg commented 8 years ago

It occurs to me that instead of enable_xhgui_new, I could look at the entity manager type and behave one way for documents and the other for entities... out of time for this project for now though.

jonaswouters commented 8 years ago

hey @aheinz-sg

Any idea when you got the time for it? :)

thanks

aheinz-sg commented 8 years ago

Not sure why Travis is complaining. Tests are passing for me with:

$ php --version
PHP 5.5.29 (cli) (built: Sep  3 2015 11:00:33) 
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2015 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2015, by Zend Technologies
    with Xdebug v2.2.5, Copyright (c) 2002-2014, by Derick Rethans
$ composer show --installed
doctrine/annotations         v1.2.7             Docblock Annotations Parser
doctrine/cache               v1.4.2             Caching library offering an object-oriented API for many cache backends
doctrine/collections         v1.3.0             Collections Abstraction library
doctrine/common              v2.5.1             Common Library for Doctrine projects
doctrine/inflector           v1.0.1             Common String Manipulations with regard to casing and singular/plural rules.
doctrine/lexer               v1.0.1             Base library for a lexer that can be used in Top-Down, Recursive Descent Parsers.
facebook/xhprof              dev-master d9bfac3 XHProf: A Hierarchical Profiler for PHP
phpspec/prophecy             1.1.2              Highly opinionated mocking framework for PHP 5.3+
phpunit/php-code-coverage    2.2.3              Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator    1.3.4              FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template    1.2.1              Simple template engine.
phpunit/php-timer            1.0.7              Utility class for timing
phpunit/php-token-stream     1.4.8              Wrapper around PHP's tokenizer extension.
phpunit/phpunit              4.1.6              The PHP Unit Testing framework.
phpunit/phpunit-mock-objects 2.1.5              Mock Object library for PHPUnit
psr/log                      1.0.0              Common interface for logging libraries
sebastian/comparator         1.2.0              Provides the functionality to compare PHP values for equality
sebastian/diff               1.3.0              Diff implementation
sebastian/environment        1.3.2              Provides functionality to handle HHVM/PHP environments
sebastian/exporter           1.2.1              Provides the functionality to export PHP variables for visualization
sebastian/recursion-context  1.0.1              Provides functionality to recursively process PHP variables
sebastian/version            1.0.6              Library that helps with managing the version number of Git-hosted PHP projects
symfony/symfony              v2.7.5             The Symfony PHP framework
twig/twig                    v1.22.2            Twig, the flexible, fast, and secure template language for PHP
$ phpunit

==== Redirecting to composer installed version in vendor/phpunit ====

PHPUnit 4.1.6 by Sebastian Bergmann.

Configuration read from /home/sg/vendor/xhprofbundle/phpunit.xml

...............

Time: 472 ms, Memory: 9.50Mb

OK (15 tests, 3 assertions)
jonaswouters commented 8 years ago

I'll have a look tomorrow. https://travis-ci.org/jonaswouters/XhprofBundle/jobs/82979994

jonaswouters commented 8 years ago

hey @aheinz-sg

I've checked locally and I'm missing dependencies for the following uses ''' use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\Common\Persistence\ManagerRegistry; '''

Can't create a mock of a class that does not exist.

aheinz-sg commented 8 years ago

I had a stale composer.lock locally. Whoops!

jonaswouters commented 8 years ago

Tagged it with v1.1.0